Skip to content

[GEN][ZH] Downgrade debug assertion to log for isPathInDirectory #1128

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

Merged
merged 2 commits into from
Jun 21, 2025

Conversation

slurmlord
Copy link

@slurmlord slurmlord commented Jun 20, 2025

#1058 added a new check for testing if a file is under a specific base path. In case it was not, a DEBUG_CRASH was triggered, before returning false.
This leads to an assert when starting the shell map, as the code in GameLogic::startNewGame explicitly sanity checks that the map is not in the save directory.
This type of sanity checking should not cause assertion failures or debug logging, so it's better to move the responsibility for reacting to those cases to the caller. This change removes the DEBUG_CRASH from FileSystem::isPathInDirectory, downgrades is to a DEBUG_LOG and places it in GameState::portableMapPathToRealMapPath.

@slurmlord slurmlord changed the title [GEN[ZH] Downgrade debug assertion to log for isPathInDirectory [GEN][ZH] Downgrade debug assertion to log for isPathInDirectory Jun 20, 2025
@xezon
Copy link

xezon commented Jun 20, 2025

Wouldn't that mean, this Debug Log is printed for start a new game for any (official) map that is in the Game Install directory (in MapsZH.big)? Is that the way it should be?

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Debug Is mostly debug functionality Fix Is fixing something Refactor Improves the structure of the code, with negligible changes in function. and removed Fix Is fixing something labels Jun 20, 2025
@slurmlord
Copy link
Author

Yes, I agree, that would be needless log entries that causes more confusion for someone reading the logs than help.
I think the logging should be removed from the "false"-case in isPathInDirectory and instead just move something similar to it into the "false"-case in GameState::portableMapPathToRealMapPath:

	if (!FileSystem::isPathInDirectory(prefix, containingBasePath))
	{
		DEBUG_LOG(("Error here"));
		return AsciiString::TheEmptyString;
	}

That way it's only logged if the file is not in the expected path instead of "not being in an unexpected path" like the case now.

@xezon xezon merged commit cc7dbee into TheSuperHackers:main Jun 21, 2025
16 checks passed
@slurmlord slurmlord deleted the removePathInDirectoryAssert branch June 21, 2025 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Debug Is mostly debug functionality Minor Severity: Minor < Major < Critical < Blocker Refactor Improves the structure of the code, with negligible changes in function.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants