Skip to content

Conversation

CharlieHess
Copy link

I've got a fork where I've refactored this and put in a variety of improvements for my game. This PR is accidentally all Zenject, so ignore the diff—the gist of it can be seen here or in the last commit: The actual code changes to IsoSpriteSorting.

What does it do?

Adds support for lines of any length, and uses some trigonometry to compare points with line segments. This lets us configure sprites that would have previously required splitting, like this escalator:

Screen Shot 2022-08-29 at 8 59 58 PM

Screen Shot 2022-08-29 at 9 00 37 PM

What else?

It fixes a nasty sorting bug that resulted from trying to track the static sprites separate from movable sprites. There's actually no need for this separation, at least not in my testing—we can keep them all in the same list and rely on transform.hasChanged to identify the sprites that are moving efficiently. We can then remove the isMovable property and greatly simplify some of the topological sorting code.

What else???

Some quality of life improvements:

  • Implement IComparable to simplify the built-in List.Sort
  • Add the fix from @rskew around renderer bounds
  • Use LINQ for enumeration wherever possible
  • Simplify naming and add comments here and there

@CharlieHess CharlieHess changed the title Refactor + variety of improvements Support lines with any number of points Aug 30, 2022
@CharlieHess
Copy link
Author

CharlieHess commented Aug 30, 2022

@markv12 I actually think we should put this in the asset store (for free naturally), because there's nothing this robust for handling isometric sprites and I wish I had found it sooner. Let me know if that's OK with you!

@markv12
Copy link
Owner

markv12 commented Sep 1, 2022

Thank you for showing interest in the project. Some of the feature additions look good.
Switching to LINQ significantly affects system's performance. The test scene only has a few objects in it and it's already generating 27kb of garbage every frame. Our game has hundreds of iso sorting objects running at a time and performance is critical.
I added 200 objects to the scene and now it's generating 16mb of garbage per frame and it's running at 15fps.
LINQ makes things more readable and I admit some of my code is pretty convoluted, but I tried to keep it as clean as possible without sacrificing performance.

The distinction between moving and non-moving sprites is also mostly for performance.

This is more of a personal preference, but I don't see a ton of value to adding something like Zenject.
I prefer to keep my frameworks as simple as possible and not require users to include a bunch of extra stuff.

@CharlieHess
Copy link
Author

@markv12 what are you using to profile LINQ like that? I had no idea it would make such a huge difference with the latest C# and some of its optimizations.

Re: moving and non-moving, that optimization can be kept, you just don't need a separate list to do it. transform.hasChanged tells you already. ☺️

Re: Zenject yeah, that's an artifact of implementing this in my game. Definitely not necessary here, and the old-school singleton can be put back easily.

@zfh2773333333
Copy link

This is awesome, and it's probably the ultimate in complex isometric sprite sorting, but it needs to be optimized!
I tried removing all linq, but still ended up with 6kb of GC per frame in the demo scene.
If you can optimize this, it would be much appreciated!

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.

3 participants