Skip to content

Conversation

@pcen
Copy link
Contributor

@pcen pcen commented Aug 7, 2023

Object IDs are discussed here: #210
Also here for save format: #377

This change is a prerequisite for implementing something like my save format draft #413 and I intend this to be an incremental change towards an actual PR for getting working save game. Having said that, regardless of how save game is implemented in the future, this PR refactors and removes string-based GUID for units and cities, and an ad-hoc ID system that was being used for tiles. The ID class consists of a string key and a unique integer value. I did not implement more complex custom ID's that would allow for things like - for cities, since this would require more complex ID parsing from saves, but can always be added in the future as a refactor of the ID class (or probably making it an interface...).

This change should not affect gameplay.

@pcen pcen changed the base branch from Development to godot4 August 7, 2023 10:59
@pcen pcen changed the title Pcen/incrementing Incrementing IDs Aug 7, 2023
@pcen pcen marked this pull request as ready for review August 7, 2023 11:38
@pcen pcen force-pushed the pcen/incrementing-id branch from aa8bf2d to 34df044 Compare September 18, 2023 02:07
@pcen pcen changed the base branch from godot4 to pcen/use-tilemaps September 18, 2023 02:07
@pcen pcen mentioned this pull request Sep 18, 2023
Copy link
Contributor

@Sean-Brown Sean-Brown left a comment

Choose a reason for hiding this comment

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

After reading the discussions, the code changes make sense

@QuintillusCFC QuintillusCFC added tilemap PRs targeting Tilemaps on Godot 4 save-game PRs/issues related to save games labels Mar 22, 2024
Copy link
Member

@QuintillusCFC QuintillusCFC left a comment

Choose a reason for hiding this comment

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

Makes sense to me as well. I'm going to see if this can be merged into the godot4 branch, keeping it from depending upon the tilemaps.

@QuintillusCFC QuintillusCFC changed the base branch from pcen/use-tilemaps to godot4 April 14, 2024 05:32
@QuintillusCFC QuintillusCFC changed the base branch from godot4 to pcen/use-tilemaps April 14, 2024 05:32
@QuintillusCFC
Copy link
Member

Nope, GitHub is not smart enough to figure out that I'd like to essentially rebase or cherry-pick to a different base. I will see if I can do that locally and thus allow decoupling of saves from tilemaps to proceed.

@QuintillusCFC
Copy link
Member

Okay, well, I accidentally cherry-picked right to godot4 rather than a branch to open a PR to merge into godot4, but at any rate this is now merged into godot4. I'll merge it into the tilemaps as originally intended as well, so it's clear that this PR is all merged.

@QuintillusCFC QuintillusCFC merged commit 62c2795 into pcen/use-tilemaps Apr 14, 2024
@QuintillusCFC QuintillusCFC deleted the pcen/incrementing-id branch April 14, 2024 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

save-game PRs/issues related to save games tilemap PRs targeting Tilemaps on Godot 4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants