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

Fixed Random-Camouflage Path Handling for Alternate Directory Format #4896

Merged
merged 4 commits into from
Sep 26, 2024

Conversation

IllianiCBT
Copy link
Collaborator

@IllianiCBT IllianiCBT commented Sep 26, 2024

Accounted for backslash escape sequences in camouflage file paths to ensure compatibility across different operating systems. Added an alternate root directory constant and updated the path sanitization to replace the alternate format correctly.

Closes #4895

Accounted for backslash escape sequences in camouflage file paths to ensure compatibility across different operating systems. Added an alternate root directory constant and updated the path sanitization to replace the alternate format correctly.
@IllianiCBT IllianiCBT added the Bug label Sep 26, 2024
@IllianiCBT IllianiCBT self-assigned this Sep 26, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.45%. Comparing base (caef4d9) to head (d355253).
Report is 51 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #4896      +/-   ##
============================================
- Coverage     10.46%   10.45%   -0.02%     
+ Complexity     6017     6009       -8     
============================================
  Files           952      952              
  Lines        132755   132758       +3     
  Branches      19308    19308              
============================================
- Hits          13894    13880      -14     
- Misses       117516   117528      +12     
- Partials       1345     1350       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@IllianiCBT
Copy link
Collaborator Author

IllianiCBT commented Sep 26, 2024

This should not be merged until someone on a windows system confirms it is working as intended. Action

This can be tested by starting a new campaign, picking a contract from the market (any contract), and then editing that contract. If camo has been randomly generated then this fix works. If it's showing a red 'X' we need to explore other options.

Corrected the typo in the file path from "mages" to "images". This ensures the application correctly locates the camouflage directory when sanitizing the file path.
@HammerGS
Copy link
Member

Tested the artifact this AM

image

Some oddites in the logs
megamek.log
mekhq.log

@IllianiCBT
Copy link
Collaborator Author

Can you DM me a save, I want to verify the issue is the same and not something else that just happens to give the same results

Refactored path sanitization by removing the alternative root directory string. Simplified the file path replacement logic to handle any path separators in a unified manner. This reduces code complexity and potential errors.
Replaced backslashes with forward slashes when determining file categories. This ensured consistency and fixed potential issues on non-Windows platforms.
@IllianiCBT
Copy link
Collaborator Author

QA have confirmed this is working now, so it should be good to merge whenever

@HammerGS HammerGS merged commit 5337910 into MegaMek:master Sep 26, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[0.50.01 Nightly] New contract has no camo or placeholder colour for Allies or Enemies
3 participants