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

Multiple tile objects on a single tile #1269

Merged
merged 15 commits into from
Nov 4, 2023

Conversation

stilnat
Copy link
Contributor

@stilnat stilnat commented Sep 29, 2023

Summary

Implementing the ability to add multiple tile objects on a single tile, for low and high wall mount. Can easily be changed to work for other layers. Let me know if it's necessary. For now, tile layer is fully defining how much item a tile location can contain. Could potentially be changed in the future.

Pictures/Videos (optional)

2023-09-29.20-23-59.mp4

Changes to Files

TileObject names were changed to TileLocation. It just makes more sense. Previously, a TileObject could contain a PlacedTileObject. Now, a TileLocation can contain a PlacedTileObject, sounds better to me.

Technical Notes (optional)

The whole thing works using an interface ITileLocation.
SingleTileLocation are for Tile locations that can contain a single tile object.
Cardinal Tile Location are for tile locations that can contain a tile object in all 4 cardinal direction.

The interface allows potentially to implement a AllDirectionTileLocation for 8 tile objects.

This choice is because most tiles are Single tile locations, and I didn't wanted to have a single class able to support up to 8 directions, it's too heavy and takes too much memory. The interface allows light implementation for single object location and heavier one for cardinal ones.

Known issues

To allow for polymorphic serialization, I had to use the SerializeReference attribute in saved tile chunk. It's not really bad, but the save files are bigger than what they could be. It makes the code really cleaner though, compared to having to deal with each type implementing ISavedTileLocation.

Fixes (optional)

Closes #1104

TODO

  • removing an object in a cardinal tile location should not remove all objects of that tile.
  • Comment the new code.
  • Redo the old map more or less.

@stilnat stilnat changed the title [WIP] Multiple tile objects on a single tile Multiple tile objects on a single tile Oct 31, 2023
Copy link
Member

@cosmiccoincidence cosmiccoincidence left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only thing I cant really comment on is the quality of the code, but the logic looks good, functions properly, no errors, changes to files are appropriate, etc..

(Don't forget to update the gitbook docs once this is merged. I'd maybe even suggest we start enforcing gitbook prs to be made in parallel with github prs for changes like this, and have the gitbook pr linked in the description of the github pr.)

@stilnat
Copy link
Contributor Author

stilnat commented Nov 2, 2023

@cosmiccoincidence I agree with enforcing doc when necessary. I suggest after PR is approved and before it is merged, so that we don't write docs for unaccepted PRs.

@joaoburatto joaoburatto merged commit a98d96f into RE-SS3D:develop Nov 4, 2023
2 checks passed
@cosmiccoincidence cosmiccoincidence added the Tilemaps Tasks related to the tilemap system or its connectors. label Nov 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tilemaps Tasks related to the tilemap system or its connectors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Multiple wallmounts per layer
3 participants