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

optimization: initialize game directory only once and assign personal ID to it #550

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

SmileyAG
Copy link
Collaborator

I didn't have time to check if there were any typos or mixed up IDs, so I'll take a look again after likely today later and give a final comment.

For now, I've requested it for the sake of reviewing the implementation as a whole. Of course, ideally, I'd like to split some of the changes into several pull requests, but then the review and their rebase will take even longer, and you want a release as soon as possible, so you'll have to accept it as it is now. Yeah...

Compared to the old code:

  • Gamedir is initialized only once! (including path cleanup and lowercase conversion)

Before this, all these actions occurred with each call of a function somehow related to the game directory, which of course you understand how it can affect performance.

  • We now check not the strings of game directories, but instead we make them personal IDs and compare them by them, it is more convenient to work with this, and besides, it should also add to performance.

@SmileyAG SmileyAG added the high-priority Pull request or issue that should be reviewed and merged first. label Aug 12, 2024
@@ -97,7 +97,7 @@ typedef enum
GAMEDIR_MATCH_TWHLTOWER2, // twhltower2 (TWHL Tower 2)
GAMEDIR_MATCH_URBICIDE, // hl_urbicide (Half-Life: Urbicide)
GAMEDIR_MATCH_VISITORS, // visitors (Half-Life: Visitors)
GAMEDIR_MATCH_HLRATS_PARASOMNIA // hrp (Half-Rats: Parasomnia)
GAMEDIR_MATCH_HLRATS_PARASOMNIA, // hrp (Half-Rats: Parasomnia)
Copy link
Owner

Choose a reason for hiding this comment

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

Squash these into the right commit pls

}
}

if (lowercase) ? return gamedir_lw; : return gamedir;
Copy link
Owner

Choose a reason for hiding this comment

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

Cursed statement of the day

int GameDirMatchID = -1, GameDirStartsWithID = -1;
inline void GameDirInit() { if (gamedir.empty()) GetGameDir(); }
inline bool _IsGameDirMatch(int val) { GameDirInit(); if (val == GameDirMatchID) ? return true; : return false; }
inline bool _IsGameDirStartsWith(int val) { GameDirInit(); if (val == GameDirStartsWithID) ? return true; : return false; }
Copy link
Owner

Choose a reason for hiding this comment

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

Also broken if statements

len = end - start + 1;
}

void com_filebase(const char *in, char *out)
Copy link
Owner

Choose a reason for hiding this comment

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

Is this needed in regular char form?


return std::strstr(map_name, map);
Copy link
Owner

Choose a reason for hiding this comment

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

Why? It's called "DoesMapNameContain" not "DoesMapNameStartWith"

@@ -6860,14 +6866,25 @@ std::string GetGameDir(bool lowercase)
}
}

if (lowercase) ? return gamedir_lw; : return gamedir;
if (lowercase)
Copy link
Owner

Choose a reason for hiding this comment

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

These changes sound like they also need to be in the last commit (or vice versa). At the very least to remove clearly broken code from commits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority Pull request or issue that should be reviewed and merged first.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants