Skip to content

chore(script): add safety check for identical files during unification and move files #2579

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.

Sign up for GitHub

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

Closed
DevGeniusCode wants to merge 2 commits into from
+13 −0
Closed

chore(script): add safety check for identical files during unification and move files #2579

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
871d553
chore(script): add safety check for identical files during unificatio…
DevGeniusCode Apr 11, 2026
e5b601d
Fix FileNotFoundError
DevGeniusCode Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions scripts/cpp/unify_move_files.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import os
import shutil
import filecmp
Copy link
Copy Markdown

@xezon xezon Apr 11, 2026

Choose a reason for hiding this comment

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

sort by alphabet

from enum import Enum


Expand Down Expand Up @@ -79,6 +80,18 @@ def unify_file(fromGame: Game, fromFile: str, toGame: Game, toFile: str):
fromGamePath = get_game_path(fromGame)
toGamePath = get_game_path(toGame)

# Safety Identical Check
fullFromPath = os.path.join(fromGamePath, os.path.normpath(fromFile))
fullOppositePath = os.path.join(fromOppositeGamePath, os.path.normpath(fromFile))

if not os.path.exists(fullFromPath) or not os.path.exists(fullOppositePath):
print(f"ERROR: One or both files do not exist! Skipping: {fromFile}")
return
Copy link
Copy Markdown

@xezon xezon Apr 11, 2026

Choose a reason for hiding this comment

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

Previously it errored when a file was not found, stopping the entire script. This was intentional, to make very clear that there is an invocation error that needs addressing before continuing.

I suspect this test was added to so that it is possible to rerun the script without resetting the invocation setup?

This should then also be done in the other unify functions.

Better make this a separate helper function similar to move_file


if not filecmp.cmp(fullFromPath, fullOppositePath, shallow=False):
Copy link
Copy Markdown

@xezon xezon Apr 11, 2026

Choose a reason for hiding this comment

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

Better make this a separate helper function similar to move_file

Copy link
Copy Markdown

@xezon xezon Apr 11, 2026

Choose a reason for hiding this comment

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

Does this really work correctly? Typically the first lines are different between Generals and Zero Hour source files, because of the Copyright header (intentional).

Copy link
Copy Markdown
Author

@DevGeniusCode DevGeniusCode Apr 11, 2026

Choose a reason for hiding this comment

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

I think the difference lies in how we merge the files. I first compare the files using the diff window and adjust any mismatches (including the first few lines) so that the Vanilla and MD files are completely identical. Only then do I transfer them

Copy link
Copy Markdown

@xezon xezon Apr 11, 2026

Choose a reason for hiding this comment

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

Merging the Copyright header in the first line is incorrect (!)

Generals must keep its Generals header, otherwise git will have difficulties resolving the rename and will pick Generals or Zero Hour at random, but we want git to always assume that Generals file is deleted and Zero Hour is moved to Core. Reason: This makes merging conflicts easier, because all our WIP changes touch Zero Hour first.

Copy link
Copy Markdown
Author

@DevGeniusCode DevGeniusCode Apr 11, 2026

Choose a reason for hiding this comment

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

Got it, that makes perfect sense. I was trying to manually resolve all differences (comments, whitespace, etc.) to make the files identical before moving them, but I see how this interferes with Git's tracking. Moving forward, I’ll make sure the Core files remain exact copies of the Zero Hour version to preserve the history. Thanks!

print(f"ERROR: Files are not identical! Skipping to prevent data loss: {fromFile}")
return

fromFirstFolderIndex = fromFile.find("/")
toFirstFolderIndex = toFile.find("/")
assert(fromFirstFolderIndex > 0)
Expand Down
Loading