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

Allow multiple objects in cell #48

Merged
merged 15 commits into from
Aug 16, 2023

Conversation

ZivDero
Copy link
Collaborator

@ZivDero ZivDero commented Aug 9, 2023

This PR converts Waypoint, Aircraft, Vehicle and Structure into Lists, allowing to place multiple of those entities in a single cell, which is supported by the game engine.

Copy link
Member

@Rampastring Rampastring left a comment

Choose a reason for hiding this comment

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

The game does not fully support this functionality. In particular, if buildings placed on top of each other, the game crashes if one of them is destroyed. This was a common mapping error with FinalSun/FinalAlert, which WAE was designed to prevent.

There are still a couple of good reasons for in turning a cell's buildings into a list though:

  1. it has use for buildings that are converted to overlay. For example, placing a crate building with ToOverlay=CRATE under a real destroyable building, to add a collectible crate that the player can pick up after destroying the building
  2. supporting maps that already have overlapping buildings

However I don't think WAE should allow placing overlapping buildings by default. Instead this should be behind some modifier key, so the mapper can't accidentally make buildings overlap, but instead it's always a deliberate action.

For units, aircraft and waypoints there is no such issue AFAIK, but is it beneficial to allow them to overlap? I have a hard time imagining a case where making units overlap would be very useful. It could still be allowed with the modifier key, but I'd also prevent it by default.

For the implementation, I think the current implementation might crash when deleting objects as you're modifying a collection that is being foreach'd. Deletion should instead act on a copy of the list.

@ZivDero
Copy link
Collaborator Author

ZivDero commented Aug 9, 2023

YR does in fact fully support this and does not crash (whether the buildings are invincible or not). Although it seems that invincile buildings also get damaged when the vulnerable ones receive damage. I suppose adding a modifyer key wouldn't hurt, though.

As for deletion, in my tests it did not crash but I agree that this is not a safe way to do it indeed so will redo.
EDIT: In fact, it should be safe as I'm not performing actions on the collecttion that's being foreached, rather on a different collection.

@ZivDero
Copy link
Collaborator Author

ZivDero commented Aug 13, 2023

Okay, all done. I've added Alt as the default hotkey that allows to place overlapping objects. Works for placing new buildings, moving/cloning buildings, as well as the Terrain copy tool. Don't think I've missed anything.

Copy link
Member

@Rampastring Rampastring left a comment

Choose a reason for hiding this comment

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

Functionality looks good now.

There are some stylistical things to fix, including some harmful renames.

src/TSMapEditor/Models/MapTile.cs Outdated Show resolved Hide resolved
src/TSMapEditor/Initialization/MapLoader.cs Outdated Show resolved Hide resolved
src/TSMapEditor/Initialization/MapLoader.cs Outdated Show resolved Hide resolved
src/TSMapEditor/Models/Map.cs Outdated Show resolved Hide resolved
@ZivDero
Copy link
Collaborator Author

ZivDero commented Aug 16, 2023

Implemented the requested changes.

@Rampastring Rampastring merged commit a762f25 into CnCNet:master Aug 16, 2023
1 check passed
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