Skip to content

Issue #782 Using shapely to find line of sight #783

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

Conversation

akapkotel
Copy link
Contributor

What is wrong with it? How can it be improved?
#782
Current has_line_of_sight function works, but it is not so efficient. With just 25 sprites and 25 obstacles if each sprite try to find line of sight to each other, it takes roughly 7 seconds for frame to update! With Shapely it is just 0.18 second: 35 times faster! With 1 player-sprite checking line of sight against 1156 enemies, it currently takes about 16 seconds per frame, whereas shapely does the job within 0.09 second

@pvcraven
Copy link
Member

pvcraven commented Nov 6, 2020

Thanks for the PR! That sounds very promising.

I'm not processing these PRs as fast as I'd like, so thanks for your patience on this.

@akapkotel
Copy link
Contributor Author

@pvcraven sure, no problem! I've just realised, that there is another, potential gain of implementing shapely: it does not require SpriteLists to use spatial-hashing for efficiency, so moving Sprites could be tested for visibility with the same efficiency as static ones!

I use shapely in my RTS-python-game project with a good results.

@pvcraven
Copy link
Member

pvcraven commented Nov 6, 2020

Oh nice. Teaching is occupying a lot of my time right now, but once break hits, I want to dive into this.

@pvcraven
Copy link
Member

pvcraven commented Nov 7, 2020

We do need to be more careful about the version number of our libraries, but I'm not sure this is the PR to do that in. Thinking maybe that should be separated.

@einarf
Copy link
Member

einarf commented Nov 7, 2020

Yes, I would revert the new version constraints for now. That needs to be carefully looked at.

@Cleptomania
Copy link
Member

Cleptomania commented Nov 7, 2020

My PR #780 deals with the package versioning issues, and is probably the more appropriate place for that to be handled since it re-does the whole setuptools configuration.

It does only deal with the versions for core packages though, like pyglet, pillow, pytiled_parser, etc. It does not do it for the dev dependencies like pytest.

@pvcraven
Copy link
Member

pvcraven commented Nov 7, 2020

With a simple test I was getting 4.77 ms with the shapely code, and 8.55 ms without shapely, but with spatial hashing, and 101 ms without spatial.

This makes me also want to explore using shapely for collision detection. Does that work?

Do we want shapely as a dependency?

@pvcraven
Copy link
Member

pvcraven commented Nov 7, 2020

Real quick error-prone test:

  • Intersection with no spatial hash: 13 ms
  • Intersection with spatial hash 8 ms
  • Shapely intersection code: 6.6 ms

@einarf
Copy link
Member

einarf commented Nov 7, 2020

Worst case there could be a shapely implementation and a pure python one? I'm not sure if that is too much to maintain.

@pvcraven
Copy link
Member

pvcraven commented Nov 7, 2020

I wonder if this also fixes issue #771 as an alternative to PR #776.

@pvcraven
Copy link
Member

pvcraven commented Nov 7, 2020

If you can update the PR to not change so much in the requirements, that would be great. I'll merge it in.

@akapkotel
Copy link
Contributor Author

@pvcraven yup, shapely also has collisions for polygons, both convex and concave etc. So it could help with collisions-handling too. I can explore that, test efficiency and if it works faster than current collision-detection, start work on bigger commit. Should I? It seems that adding shapely-depenedence would be a good idea.

@pvcraven
Copy link
Member

pvcraven commented Nov 7, 2020

Let's do that as a separate PR. If this one has requirements.txt cleaned up, I'll put it in for 2.5, then do a different PR for working collision detection.

1. shapely and setuptools removed from requirements.txt
@akapkotel
Copy link
Contributor Author

@pvcraven I removed requirements.

removed all versioning.
@pvcraven pvcraven merged commit 9844fba into pythonarcade:development Nov 7, 2020
@akapkotel akapkotel deleted the using_shapely_to_find_line_of_sight branch November 10, 2020 12:02
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