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

Remove raw string item constructor #79056

Merged

Conversation

Procyonae
Copy link
Contributor

@Procyonae Procyonae commented Jan 10, 2025

Summary

None

Purpose of change

Found a todo that seemed simple enough to give me a break from getting very confused by diagonal rail intersection sprite rotations

Describe the solution

Mainly just changes the old raw string constructor usage with itype_id's statically defined at the top in the standard way
Changes a couple of knock on functions/fields using raw strings when they shouldn't be
Changes npc_talks receive_item bc it was passing a itype_id and then conditionally reparsing it as an item_group_id which seemed really hacky

Describe alternatives you've considered

Same wants doing for a few other types notably monsters
I'm not a fan of harvest_entry::drop which is a raw string which can be treated as a itype_id or item_group_id depending on the associated json harvest_drop_type's "group" field but that didn't seem super simple to change

Testing

Game and tests compile, will do a full review once the tests have ran... or Guardian could just yolo it '^^

Additional context

This was a 20 minute adventure until I got to /tests .-.

@github-actions github-actions bot added NPC / Factions NPCs, AI, Speech, Factions, Ownership Info / User Interface Game - player communication, menus, etc. Items: Magazines Ammo holding items and objects. Missions Quests and missions Bionics CBM (Compact Bionic Modules) Map / Mapgen Overmap, Mapgen, Map extras, Map display Vehicles Vehicles, parts, mechanics & interactions Items: Food / Vitamins Comestibles and drinks Items: Battery / UPS Electric power management Crafting / Construction / Recipes Includes: Uncrafting / Disassembling Code: Tests Measurement, self-control, statistics, balancing. [C++] Changes (can be) made in C++. Previously named `Code` Monsters Monsters both friendly and unfriendly. Fields / Furniture / Terrain / Traps Objects that are part of the map or its features. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Melee Melee weapons, tactics, techniques, reach attack Player Faction Base / Camp All about the player faction base/camp/site Mechanics: Weather Rain, snow, portal storms and non-temperature environment Items: Archery Bows, crossbows, arrows, bolts Items: Containers Things that hold other things Mechanics: Enchantments / Spells Enchantments and spells Appliance/Power Grid Anything to do with appliances and power grid labels Jan 10, 2025
@github-actions github-actions bot added the EOC: Effects On Condition Anything concerning Effects On Condition label Jan 10, 2025
@github-actions github-actions bot requested a review from KorGgenT January 10, 2025 16:48
@GuardianDll
Copy link
Member

Do we want to enforce it somehow, to prevent it from occuring in future?
Also please resolve branch conflicts

@Procyonae
Copy link
Contributor Author

Procyonae commented Jan 10, 2025

Do we want to enforce it somehow, to prevent it from occuring in future?

I removed the constructor overload (the first commit) so unless someone tried to add one back that's not an issue

@github-actions github-actions bot added json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Jan 10, 2025
src/item.h Outdated Show resolved Hide resolved
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Jan 11, 2025
@GuardianDll GuardianDll merged commit 5bfca4e into CleverRaven:master Jan 11, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Appliance/Power Grid Anything to do with appliances and power grid astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions Bionics CBM (Compact Bionic Modules) [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. Items: Ammo / Guns Ammunition for all kinds of weapons and these weapons themselves Items: Archery Bows, crossbows, arrows, bolts Items: Battery / UPS Electric power management Items: Containers Things that hold other things Items: Food / Vitamins Comestibles and drinks Items: Magazines Ammo holding items and objects. json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display Mechanics: Enchantments / Spells Enchantments and spells Mechanics: Weather Rain, snow, portal storms and non-temperature environment Melee Melee weapons, tactics, techniques, reach attack 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 Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants