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

TiledMapResource#getTilesetForTile works incorrectly #380

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

HxShard
Copy link
Contributor

@HxShard HxShard commented Jun 23, 2022

Now it takes first tile set with firstGid < tileGid, even if this tile is out of range.
I have 19 tile sets in my project, and TiledMapResource#getSpriteForGid (which is based on TiledMapResource#getTilesetForTile) works 50/50: reloading page give different results. But before, with only 5 tile sets, everything worked just like a charm. I found that problem was caused by this method. Thank you!

@eonarheim
Copy link
Member

@HxShard Thanks for the PR, this 100% makes sense to add.

We must have avoided this issue in the past with embedded tilesets in order of first gid, or low numbers (like only 1) of external tilesets 🤯

@eonarheim eonarheim merged commit 474dd3c into excaliburjs:main Jun 23, 2022
@HxShard
Copy link
Contributor Author

HxShard commented Jun 23, 2022

@eonarheim Sorry, I made a mistake here :((

firstGid starts from 1, while ID of first tile is always 0
In tile set with 100 tiles, ID of last tile is 99

So I think, we need to make first condition look like this:
gid >= ts.firstGid && gid <= ts.firstGid + ts.tileCount - 1

Just tested it many times, exactly with -1 function works correctly.

@eonarheim
Copy link
Member

Thanks for pointing it out 👍 please raise a new PR when you get a chance

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.

2 participants