optimization: initialize game directory only once and assign personal ID to it#550
Open
SmileyAG wants to merge 6 commits into
Open
optimization: initialize game directory only once and assign personal ID to it#550SmileyAG wants to merge 6 commits into
SmileyAG wants to merge 6 commits into
Conversation
SmileyAG
added
the
high-priority
YaLTeR
requested changes
Sep 16, 2024
| 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) |
Owner
There was a problem hiding this comment.
Squash these into the right commit pls
| } | ||
| } | ||
|
|
||
| if (lowercase) ? return gamedir_lw; : return gamedir; |
| 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; } |
| len = end - start + 1; | ||
| } | ||
|
|
||
| void com_filebase(const char *in, char *out) |
Owner
There was a problem hiding this comment.
Is this needed in regular char form?
| ConvertToLowerCase(map_name); | ||
| helper_functions::convert_to_lowercase(map_name); | ||
|
|
||
| return std::strstr(map_name, map); |
Owner
There was a problem hiding this comment.
Why? It's called "DoesMapNameContain" not "DoesMapNameStartWith"
| } | ||
|
|
||
| if (lowercase) ? return gamedir_lw; : return gamedir; | ||
| if (lowercase) |
Owner
There was a problem hiding this comment.
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
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
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:
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.