Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update/kivent revival: Take 2! #281

Merged
merged 18 commits into from
Oct 18, 2019
Merged

Conversation

tim-win
Copy link
Contributor

@tim-win tim-win commented Oct 13, 2019

Major Changes required to get KivEnt back up and running:

on_kv_post

KivEnt examples almost all follow this pattern:

class TestGame(Widget):
    def __init__(self, **kwargs):
        super(TestGame, self).__init__(**kwargs)
        self.gameworld.init_gameworld(['system_ids'], callback=self.init_game)
        ...

Combined with a .kv file that defines a gameworld:

TestGame:

<TestGame>:
	gameworld: gameworld
    GameWorld:
        id: gameworld
        ...

However, new functionality in Kivy includes moving kv rule application to a callback to give developers more control. You can see the implementation here.

This is a breaking change, which is why I recommend bumping KivEnt to version 3.0- as far as I can tell everything will need to be updated to support this new pattern. However, after adding the on_kv_post method to cwidget, the fix is very straightforward for user code: just move the init gameworld call and you're on your way:

image

All but the 0_empty_kivy_app examples were affected by this and have since been fixed.

Relative Imports

Implicit relative imports have been removed in Kivy to support newer versions of python, and KivEnt is awash with them. As an example fix:

image

This PR cleans up relative imports throughout KivEnt. Addresses #265 and #209

Print > Logger, when Possible

Compilation was broken on python 3+ due to at least two uses of print statements in KivEnt, but several locations were cleaned up to just use the kivy Logger instead of print.

Fix the Pull Request Tests

PR tests were fixed. Travis and Appveyor both have approved this pull request.

Travis tests were moved from trusty to xenial, and the python versions were changed to 3.5, 3.6, and 3.7.

Appveyor was changed to the same python layout, and the target Kivy version upstream was pinned to 2.0.0.dev0 (rather than 1.10.1.dev0, which no longer exists in appveyor).

Also, cymunk is not correctly compiling in Windows/Python3.5+ environments. This is documented pretty well here and also discussed here. As I'm not familiar with Windows build systems, I don't have a good test environment to try to fix this, this is a long standing issue, and I have to wash my hair tonight- I decided to dodge this issue.

The KivEnt/modules/cymunk install is conditionalized out.

Other Small changes

Then a bunch of little things.

  • Bumped the major version for all modules, with KivEnt going to 3.0
    ** This change meets semver.org's recommendation for new major versions, as it is backwards incompatible with old KivEnt builds due to the on_post_kv issue
  • Fixed the exit code in the scripts/install_all.sh file, which is handy as heck.
  • Cast a few doubles and a long to ints.
    ** I couldn't determine when this change occurred, but several calls to len were returning doubles, and then being saved to integer vars in kivy or elsewhere. This choked the cythonization so I went and fixed it.
  • calls to dec_disabled were failing, because cwidget didn't implement it! Ported over the *_disabled methods from Kivy to cwidget. Addresses Kivent objects (widgets) do not have attribute 'dec_disabled' #276
  • updated requirements.txt to poist to latest versions of Kivy and stuff.git
  • Updated the 15_rotate_camera example pretty thoroughly, because I couldn't figure out if it was rotating the camera in its original form. Once I could prove the camera rotation worked, it was only a little bit of cleanup to get the example to its current form.
  • Had to remove a redeclaration of _proxy_ref. Addresses Kivent Core compilation #277

To-Do before this can be merged:

  • Get reviewed!

@tim-win
Copy link
Contributor Author

tim-win commented Oct 14, 2019

Travis is up and running- Windows is next on the docket.

Currently Cymunk doesnt install in windows/python3.5+ environments, which
is a dealbreaker for KivEnt/modules/cymunk working in modern envionments.

Remove testing of cymunk until cymunk is fixed

(see kivy/cymunk#52)
@tim-win
Copy link
Contributor Author

tim-win commented Oct 14, 2019

Cymunk looks like it has been broken in windows based python 3.5+ environments for a while:

kivy/cymunk#52

I'm not a Windows guy so I don't have great knowledge of how to just go fix it, nor do I have great test environments to make up meaningful workarounds. So I did what any good engineer does and kick the can down the road.

i.e. I conditionalized out the cymunk install.

Yes, this is a cop out, but this will allow us to testing building four of the five KivEnt modules in windows/python3.5, which is not nothing.

@jwasys
Copy link

jwasys commented Oct 14, 2019

Hello tim-win,

Thanks for the work, I'd love to try this out.

What would be a working combination of cymunk and kivent, version wise ? I cannot get anything to work here.

I have kivy-kivent-4709f07 and kivy 1.11.1

Thanks,

Arian

@tim-win
Copy link
Contributor Author

tim-win commented Oct 14, 2019

Hey jwasys!

You'll need cymunk straight from the GitHub.com/kivy/cymunk repo, and I build latest master for github.com/kivy/kivy as well. You need to build extensions for kivy in place for the kivent build to find the ,cyx files, and will also need to add the kivy and cymunk builds to your pythonpath.

Writing all of that out on a phone, maybe I should update the readme with install instructions!

@tim-win
Copy link
Contributor Author

tim-win commented Oct 14, 2019

Also, 4709f07 is incompatible with kivy 1.11, you'll need to use this branch to get it running!

@tim-win
Copy link
Contributor Author

tim-win commented Oct 16, 2019

@Kovak Can you take a look at this PR? I think it's ready for review and merge.

@tito and @matham, you have closed the last 4 PRs to kivy/kivy and seem to be active maintainers in the org. Would you also be willing to take a look? I think KivEnt is just too cool to let bit rot like this :D

Copy link
Member

@tshirtman tshirtman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't test, but from reading, it seems good to me, given the current lack of maintenance on this project, i'm all for letting people pushing it forward, so assuming it works for you, i'm ok with merging it.

Good job, very welcome.

@jwasys
Copy link

jwasys commented Oct 16, 2019

Hello tim-win

Instructions would be great since I've been swimming for too long now...

Thanks

Arian

@tim-win
Copy link
Contributor Author

tim-win commented Oct 17, 2019

Yes! A positive review!

I don't have access to merge to this repo / branch, as master is protected here, but please let me know if there's anything I can do to help.

@tim-win
Copy link
Contributor Author

tim-win commented Oct 17, 2019

Arian- Happy to update the readmes next. I think there's a fair amount of house cleaning that can be done here, but I'd like to call this PR complete for now as it does get PR tests back into shape.

@tshirtman tshirtman merged commit e22f871 into kivy:master Oct 18, 2019
@crimsonbay
Copy link

Travis is up and running- Windows is next on the docket.

I was able to build under Windows. Check core(2_basic_app) and cymunk(6_controlling_the_viewing_area).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants