Introduce character_id type #33344
Merged
Introduce character_id type #33344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ZhilkinSerg
added
[C++]
Add a new class character_id, which is just a simple wrapper around int for type-safety purposes. No semantic change intended.
jbytheway
force-pushed
the
character_id
branch
from
9918383 to
7114ff6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
SUMMARY: Infrastructure "Introduce character_id type (rather than just using int)"
Purpose of change
To provide improved type safety and code clarity.
Currently
npcandavatarget an integer id stored as anint. This is easy to misuse and unclear to work with.Describe the solution
Introduce a new type
character_idwhich is a thin wrapper aroundint. Use it in all the placescharacterids were being used.Previously there was a distinction between two types of invalid
characterid:-1was "really invalid" and0was "should have an id, but you're loading a legacy save game that didn't have them". I think such saves are 0.C-era, so I didn't maintain this distinction. I don't think it was actually an important distinction anyway; I think things should continue to work even if loading an old save, but I haven't tested that.Describe alternatives you've considered
missionids should probably be handled similarly. I could pull out the common implementation for both into a base class. Might do that if I addmission_idin a future commit.Could have tried to leverage the existing
int_idinfrastructure, but that's closely tied to loading game data from json, so I don't think it's a good fit here.Additional context
If we ever want to change the internal representation of such ids in the future, we now have fewer places we need to edit.