Skip to content

Replace NAMEKEY strings with defines #373

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ShizCalev
Copy link

WIP.

Replaces a lot of strings with defines. No behavior change, this simply hardens against typos causing silent bugs in the future.

@DevGeniusCode DevGeniusCode added Enhancement Is new feature or request Executable Is game code related labels Mar 5, 2025
@Generalcamo Generalcamo added Refactor Edits the code with insignificant behavior changes, is never user facing and removed Enhancement Is new feature or request labels Mar 6, 2025
Copy link

@xezon xezon left a comment

Choose a reason for hiding this comment

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

Change is wip but I can already add some notes :)

Perhaps the next step (in a follow up change) would be to change name key string to name key hash and have these hashes of names generated at compile time. The original string could perhaps be preserver for debug in a NameKey struct, that contains both hash and string.

@@ -1409,7 +1409,7 @@ void ControlBar::update( void )
Int count;
const ThingTemplate *thing = TheThingFactory->findTemplate( ThePlayerList->getLocalPlayer()->getPlayerTemplate()->getBeaconTemplate() );
ThePlayerList->getLocalPlayer()->countObjectsByThingTemplate( 1, &thing, false, &count );
static NameKeyType beaconPlacementButtonID = NAMEKEY("ControlBar.wnd:ButtonPlaceBeacon");
static NameKeyType beaconPlacementButtonID = NAMEKEY(NAMEKEY_ControlBar.wnd:ButtonPlaceBeacon);
Copy link

Choose a reason for hiding this comment

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

Are these related for sure?

@@ -1409,7 +1409,7 @@ void ControlBar::update( void )
Int count;
const ThingTemplate *thing = TheThingFactory->findTemplate( ThePlayerList->getLocalPlayer()->getPlayerTemplate()->getBeaconTemplate() );
ThePlayerList->getLocalPlayer()->countObjectsByThingTemplate( 1, &thing, false, &count );
static NameKeyType beaconPlacementButtonID = NAMEKEY("ControlBar.wnd:ButtonPlaceBeacon");
static NameKeyType beaconPlacementButtonID = NAMEKEY(NAMEKEY_ControlBar.wnd:ButtonPlaceBeacon);
Copy link

@xezon xezon Mar 6, 2025

Choose a reason for hiding this comment

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

The string concat is expected to be NAMEKEY_ControlBar ".wnd:ButtonPlaceBeacon" (I am not sure what NAMEKEY does) or make them explicit ones like you have for some others.

@xezon xezon changed the title Refactor: Replaces a lot of NAMEKEY strings with defines Replace NAMEKEY strings with defines Mar 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Executable Is game code related Refactor Edits the code with insignificant behavior changes, is never user facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants