-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Remove extern ter ids #72766
Merged
kevingranade
merged 16 commits into
CleverRaven:master
from
Procyonae:RemoveExternTerIds
Apr 4, 2024
Merged
Remove extern ter ids #72766
kevingranade
merged 16 commits into
CleverRaven:master
from
Procyonae:RemoveExternTerIds
Apr 4, 2024
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
github-actions
bot
added
NPC / Factions
NPCs, AI, Speech, Factions, Ownership
Missions
Quests and missions
Map / Mapgen
Overmap, Mapgen, Map extras, Map display
Vehicles
Vehicles, parts, mechanics & interactions
Crafting / Construction / Recipes
Includes: Uncrafting / Disassembling
[C++]
Changes (can be) made in C++. Previously named `Code`
Fields / Furniture / Terrain / Traps
Objects that are part of the map or its features.
Scenarios
New Scenarios, balancing, bugs with scenarios
Player Faction Base / Camp
All about the player faction base/camp/site
labels
Apr 1, 2024
github-actions
bot
added
astyled
astyled PR, label is assigned by github actions
json-styled
JSON lint passed, label assigned by github actions
labels
Apr 1, 2024
nornagon
approved these changes
Apr 1, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're a saint
kevingranade
reviewed
Apr 1, 2024
kevingranade
reviewed
Apr 1, 2024
kevingranade
reviewed
Apr 1, 2024
kevingranade
reviewed
Apr 1, 2024
kevingranade
reviewed
Apr 1, 2024
Co-authored-by: Kevin Granade <[email protected]>
github-actions
bot
added
the
Code: Tests
Measurement, self-control, statistics, balancing.
label
Apr 2, 2024
github-actions
bot
added
Monsters
Monsters both friendly and unfriendly.
EOC: Effects On Condition
Anything concerning Effects On Condition
Info / User Interface
Game - player communication, menus, etc.
labels
Apr 2, 2024
github-actions
bot
removed
the
astyled
astyled PR, label is assigned by github actions
label
Apr 2, 2024
github-actions
bot
added
the
astyled
astyled PR, label is assigned by github actions
label
Apr 2, 2024
github-actions
bot
added
BasicBuildPassed
This PR builds correctly, label assigned by github actions
and removed
BasicBuildPassed
This PR builds correctly, label assigned by github actions
labels
Apr 2, 2024
Procyonae
force-pushed
the
RemoveExternTerIds
branch
from
April 3, 2024 12:57
78556a5
to
aa309ef
Compare
github-actions
bot
added
the
BasicBuildPassed
This PR builds correctly, label assigned by github actions
label
Apr 3, 2024
This was referenced May 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
astyled
astyled PR, label is assigned by github actions
BasicBuildPassed
This PR builds correctly, label assigned by github actions
[C++]
Changes (can be) made in C++. Previously named `Code`
Code: Tests
Measurement, self-control, statistics, balancing.
Crafting / Construction / Recipes
Includes: Uncrafting / Disassembling
EOC: Effects On Condition
Anything concerning Effects On Condition
Fields / Furniture / Terrain / Traps
Objects that are part of the map or its features.
Info / User Interface
Game - player communication, menus, etc.
json-styled
JSON lint passed, label assigned by github actions
Map / Mapgen
Overmap, Mapgen, Map extras, Map display
Missions
Quests and missions
Monsters
Monsters both friendly and unfriendly.
NPC / Factions
NPCs, AI, Speech, Factions, Ownership
Player Faction Base / Camp
All about the player faction base/camp/site
Scenarios
New Scenarios, balancing, bugs with scenarios
Vehicles
Vehicles, parts, mechanics & interactions
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
None
Purpose of change
Remove everything but t_null from the ancient (Pre 0.A) int_id<ter_t> list from mapdata.h/.cpp
Extern evil idk
Describe the solution
Changes references to the extern list for static const string_id<ter_t> definitions at the top of each file that uses them in line with other ids
Uses ter_str_id::NULL_id() instead for t_null
Left a few instances of t_null that were being awkward, if someone more technically adept wants to attack them here's the list
mapdata.cpp L64
mapgen.cpp L854 L4376 L5144 L7690
regional_settings.h L100 L283
submap.h L318
Changes a few excessive checks to use unordered_set and find instead
Describe alternatives you've considered
I were going to just remove the ones that aren't referenced anymore but anothersimulacrum and kevingranade both advocated for changing to using string_id<ter_t>s per file
Keeping the ids as t_ as opposed to the currently used ter_t_ for string ids and instead changing existing ter_t_ to t_ but that's probably at least just as intrusive and other types of static id use the existing format
Not causing myself merge conflicts
Testing
Game compiles and loads fine
I wouldn't be surprised if there's some random place the int_id -> string_id causes an issue that I haven't noticed and the tests don't catch but hopefully it's nothing major
Additional context
There's alot of stuff that can be replaced with flag/ter_furn_transforms instead of needing to use hardcoded ids, might attack some in future PRs