AdamsLair / duality

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

Fix TileMap Not Knowing SourceTileSize changed to square value #693

Closed helle253 closed 6 years ago

helle253 commented 6 years ago

This PR fixes an issue where the Tile Palette Editor erroneously thinks the tilemap has not changed, when the sourceTileSize value has been changed to a new pair of equal values (e.g. 16x16, 32x32, etc), and not requiring the user to apply the changes as it should.

Further investigation is needed and will be further investigated in #689.

helle253 commented 6 years ago

Can do. I wanted to keep the changes I made to within the scope of the tilemap package, but I agree, it wouldn't hurt to make the hash function for Point2 stronger. I'll change the PR to address the issue within Point2, instead, this evening.

On Tue, Oct 16, 2018 at 11:29 AM Adam notifications@github.com wrote:

@ilexp requested changes on this pull request.

Found a minor copy-paste bug, see comment above.

That said, given your problem description in issue #689 https://github.com/AdamsLair/duality/issues/689, the core problem here seems to be the hash function in Point2 that produces very weak results in the first place - can you either change or extend your PR to address this?

For typical usage scenarios of Point2, I would imagine a (Y, 16) bit-shifted XOR (X) to produce much better results.

In Source/Plugins/Tilemaps/Core/Tilesets/Tileset.cs https://github.com/AdamsLair/duality/pull/693#discussion_r225613474:

@@ -298,8 +298,8 @@ public int GetCompileHashCode() MathF.CombineHashCode(ref hash, input.Id.GetHashCode()); MathF.CombineHashCode(ref hash, input.Name.GetHashCode()); MathF.CombineHashCode(ref hash, input.SourceData.GetHashCode());

  • MathF.CombineHashCode(ref hash, input.SourceTileSize.GetHashCode());
  • MathF.CombineHashCode(ref hash, input.SourceTileSpacing.GetHashCode());
  • MathF.CombineHashCode(ref hash, input.SourceTileSize.GetHashCode() + input.SourceTileSize.X * input.SourceTileSize.Y);
  • MathF.CombineHashCode(ref hash, input.SourceTileSpacing.GetHashCode() + input.SourceTileSize.X * input.SourceTileSize.Y);

There seems to be a copy-paste bug, as the spacing hash calculation uses the tile size struct.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AdamsLair/duality/pull/693#pullrequestreview-165251917, or mute the thread https://github.com/notifications/unsubscribe-auth/AQDL5UIGht3l0OreYyts51r8t421FOXaks5ulglSgaJpZM4XddUZ .

-- Nathan Heller Software Engineer Abbott Labs 1 St Jude Medical Dr, St Paul, MN 55117 (715) 551-3074