Skip to content

Conversation

@bobtista
Copy link

Removes dead code from the calcCRC static function in MapUtil.cpp

The function declared several variables (dirName parameter, asciiFile, tempBuf, filenameBuf) that were never used. The function only needs the fname parameter to compute the CRC of a map file.

Changes:

  • Removed unused dirName parameter from function signature
  • Removed unused local variables: asciiFile, tempBuf, filenameBuf, length
  • Removed misleading comment suggesting directory handling
  • Updated call site to pass only fname
  • Applied changes to both GeneralsMD and Generals versions

@Skyaero42
Copy link

loadmap() and loadUserMaps() still need attention

@bobtista
Copy link
Author

loadmap() and loadUserMaps() still need attention

Done. Also I found one more spot with a variable that wasn't being used, cleaned it up

Copy link

@Skyaero42 Skyaero42 left a comment

Choose a reason for hiding this comment

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

Not sure if all the refactor comments are needed. I leave that up for Xezon. Otherwise this looks good.

@bobtista
Copy link
Author

bobtista commented Nov 1, 2025

Not sure if all the refactor comments are needed. I leave that up for Xezon. Otherwise this looks good.

I agree here, it's not changing any functionality that a future dev would need to know about, I was just trying to follow contributing guidelines - I think it's better to leave out the comments

File *fp;
asciiFile = fname;
fp = TheFileSystem->openFile(asciiFile.str(), File::READ);
File *fp = TheFileSystem->openFile(fname.str(), File::READ);
Copy link

Choose a reason for hiding this comment

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

Is it correct that this does not use dirName? Does it work with User Maps?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, in the original code (main branch line 84), calcCRC declared dirName as a parameter but never used it. The function only used fname to open the file.

fname comes from getFileListInDirectory, which returns either relative (standard maps) or absolute path (user maps)

TheFileSystem->openFile() handles both eg MapCache::loadStandardMaps and MapCache::loadUserMaps

@xezon xezon added Minor Severity: Minor < Major < Critical < Blocker Refactor Edits the code with insignificant behavior changes, is never user facing labels Nov 1, 2025
@bobtista bobtista force-pushed the bobtista/refactor-calcCRC-dead-code branch from b5a91eb to df6cec2 Compare November 1, 2025 16:45
@xezon xezon changed the title refactor(map): Remove unused variables and parameter from calcCRC refactor(maputil): Remove unused variables in MapUtil Nov 1, 2025
@xezon xezon merged commit 733e3db into TheSuperHackers:main Nov 1, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Minor Severity: Minor < Major < Critical < Blocker 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.

3 participants