search

AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.41k stars 287 forks source link

Rectangular tilesets are incorrectly displayed #818

Closed Seti-0 closed 4 years ago

Seti-0 commented 4 years ago

Summary

In the duality tilemaps plugin, when using a Tileset with a margin and source tiles that are not square, there will be thin vertical holes in the rendered Tilemap.

How to reproduce

For a black camera background, the holes will appear as black lines.

I'm not sure if they will show for all camera distances, but I've never seen them not show.

Analysis

The holes are present in the output from TilesetCompiler.Compile(...), and looked like missing left/right margins of width 1. (I never checked with thicker margins)

In TilesetCompiler.cs, under TilesetCompiler.FillTileSpacing(...), the function iterates over the width of the target tile size for each margin.

Switching the width to height for the left and right margins seemed to fix the issue. (Tested on a local build of Duality.)

Attachments

ilexp commented 4 years ago

Just adding some info from the chat and links for convenience, the two lines in question are:

There may be a typo and what should be iterated on is actually targetTileSize.Y, not .X, if I understood this correctly. Looks like that could be it, but didn't verify this in code so far.

Important note: In order to release this for the current v3.x Duality versions despite being in the middle of transitioning to v4.0 on master, any PRs addressing this issue should be made to merge into the archive/v3.x branch.

Additional note for @AdamsLair/duality-core maintainers: The release can then be done by doing the regular package version update script on that branch, followed by merging it into release. The change can then be merged or cherry-picked to master to retain it when moving to v4.0.

ilexp commented 4 years ago

Switching the width to height for the left and right margins seemed to fix the issue. (Tested on a local build of Duality.)

@Seti-0 Since you already seem to have a local fix, do you want to pick up this issue and contribute it with a Pull Request?

Either way, I've flagged this as Good First Issue, since it seems to be a good opportunity for new contributors to get into the project with a first PR.

Seti-0 commented 4 years ago

I made a PR :)

I've no experience with git/github, so let me know if there is something I could/should be doing differently.

SirePi commented 4 years ago

Thanks for the PR. Will be out as soon as we figure out what happened :) Closing this in the meantime