Skip to content

✨ Add MSX/MSX2/MSX2+/turboR packer (WebMSX)#20

Open
aciderix wants to merge 1 commit intomainfrom
feature/msx-packer
Open

✨ Add MSX/MSX2/MSX2+/turboR packer (WebMSX)#20
aciderix wants to merge 1 commit intomainfrom
feature/msx-packer

Conversation

@aciderix
Copy link
Copy Markdown
Owner

@aciderix aciderix commented Mar 6, 2026

🎮 MSX Packer - Nouveau système

Ajout complet du support MSX (MSX1/MSX2/MSX2+/turbo R) via WebMSX.

Fichiers ajoutés

  • docs/pack_msx_game_fr.html — Web packer français
  • docs/pack_msx_game_en.html — Web packer anglais
  • packers/msx/pack_msx_game.py — Packer CLI Python
  • docs/data/msx/wmsx.js — Moteur WebMSX 6.0 (5.0 MB, BIOS inclus)

Fonctionnalités

  • 5 formats supportés : .rom, .mx1, .mx2, .dsk, .cas, .zip
  • 4 machines : MSX1, MSX2, MSX2+, turbo R (détection auto)
  • 6 extensions sonores : SCC-I, SCC, FM-PAC, MSX-MUSIC, MegaRAM
  • Multi-disques : Support 2+ disquettes avec barre de swap
  • 100% offline : Jeu + moteur embarqués en base64
  • Miroir local avec fallback CDN pour le moteur
  • Contrôles tactiles natifs sur mobile

Tests effectués

Jeu Format Machine
Antarctic Adventure ROM 16KB MSX1
Mon Mon Monster ROM 160KB MSX1
MSX Legends ROM 8.6MB MSX2
Arkanoid DSK 720KB MSX1
Boulder Dash CAS 18KB MSX1
1789 La Révolution 3×DSK multi MSX2

Summary by CodeRabbit

Release Notes

  • New Features
    • Added MSX Game Packer: web-based tools (English and French) and command-line utility to package MSX games into standalone HTML files for offline play.
    • Supports all common game formats with single-file and multi-disk modes, drag-and-drop upload, and automatic configuration.
    • Disk-switching interface for multi-disk games and progress tracking during generation.

- Web packers FR/EN with multi-disk support
- Python CLI packer with full format support
- WebMSX 6.0 engine (5MB, BIOS included)
- Supports ROM, DSK, CAS, MX1, MX2, ZIP formats
- 4 machine types with auto-detection
- 6 sound extension presets
- Multi-disk swap bar for 2+ DSK games
- 100% offline - game embedded as base64
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Sorry @aciderix, your pull request is larger than the review limit of 150000 diff characters

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 6, 2026

📝 Walkthrough

Walkthrough

Introduces a complete MSX game packing solution with web-based frontend UI (English and French) and Python backend CLI. Enables users to create self-contained offline HTML files from MSX games with single-file and multi-disk support, featuring drag-and-drop file selection, configuration options, progress tracking, and WebMSX engine integration.

Changes

Cohort / File(s) Summary
HTML Game Packer UI
docs/pack_msx_game_en.html, docs/pack_msx_game_fr.html
Two standalone HTML pages implementing browser-based MSX game packing interfaces. Features drag-and-drop file selection, multi-disk mode support, file metadata display, machine type and sound configuration options, sequential async progress workflow (read, encode, fetch engine, assemble HTML), engine caching with CDN fallback, HTML generation for single-file and multi-disk games, error handling, and downloadable output. Responsive layout with FAQ section and accessible UI states.
Python Packing Backend
packers/msx/pack_msx_game.py
CLI tool for generating offline HTML wrappers around MSX game files using WebMSX engine. Supports .rom, .mx1, .mx2, .dsk, .cas, .zip formats with per-type defaults. Validates inputs, derives titles from filenames, locates WebMSX engine from multiple paths, encodes games in Base64, injects into HTML templates. Includes single-disk and multi-disk modes with disk-switching UI for multi-disk games. Exports main CLI functions and utility helpers for title derivation and engine discovery.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Browser
    participant LocalStorage
    participant CDN
    participant FileSystem as File System
    participant Engine as WebMSX Engine

    User->>Browser: Select game file(s) or drag-drop
    Browser->>FileSystem: Read file data
    FileSystem-->>Browser: File binary
    Browser->>Browser: Detect type, auto-fill config
    Browser->>LocalStorage: Store user config
    User->>Browser: Click Generate
    Browser->>Browser: Encode file to Base64
    Browser->>LocalStorage: Check cached engine
    alt Engine cached
        LocalStorage-->>Browser: Return wmsx.js
    else Engine not cached
        Browser->>CDN: Fetch wmsx.js (with fallback)
        CDN-->>Browser: Engine script
        Browser->>LocalStorage: Cache engine
    end
    Browser->>Browser: Assemble HTML (embed engine + Base64 data)
    Browser->>Browser: Create downloadable blob
    Browser-->>User: Display download link & size
    User->>Browser: Click Download
    Browser->>FileSystem: Save HTML file
Loading
sequenceDiagram
    participant User
    participant CLI as Python CLI
    participant FileSystem as File System
    participant Engine as WebMSX Locator
    participant Template as HTML Template

    User->>CLI: pack_msx_game(game_path, config)
    CLI->>FileSystem: Validate game file exists
    FileSystem-->>CLI: File confirmed
    CLI->>CLI: Detect file type & validate extension
    CLI->>Engine: find_engine() from multiple paths
    Engine-->>CLI: wmsx.js location
    CLI->>FileSystem: Read game binary
    FileSystem-->>CLI: Game data
    CLI->>CLI: Encode to Base64
    CLI->>Template: Inject Base64 + engine + config
    Template-->>CLI: Generated HTML
    CLI->>FileSystem: Write output HTML
    FileSystem-->>CLI: File written
    CLI->>User: Display summary (size, path, config)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 Hoppy new packer comes to play,
MSX games now pack away!
HTML bundles, disk by disk,
Offline treasures, no network risk!
From web to Python, both ways shine, 🎮✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: adding MSX game packer support (WebMSX).
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/msx-packer

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/pack_msx_game_en.html`:
- Around line 855-856: The current sort using files.sort(function(a, b) { return
a.name.localeCompare(b.name); }) incorrectly orders numeric filenames (e.g.,
"Disk 10.dsk" before "Disk 2.dsk"); change the comparison to use numeric-aware
collation (e.g., localeCompare with the numeric option or an Intl.Collator
configured with {numeric: true, sensitivity: 'base'}) so filenames sort
naturally and ensure DISK_IMAGES[0] remains the boot disk. Locate the files.sort
call and replace the comparator with a numeric-aware localeCompare or
Intl.Collator comparator applied to a.name and b.name.
- Line 1007: SOUND_PRESETS is referenced by the sound selector expression
(soundPreset and SOUND_PRESETS[soundPreset]) but never declared in this English
HTML; add a declaration for the SOUND_PRESETS object (matching the structure
used in the French file) before any usage so SOUND_PRESETS[soundPreset] resolves
correctly—ensure the declaration appears prior to the code that builds
WMSX.PRESETS and mirrors the entries/formats from docs/pack_msx_game_fr.html so
selecting a non-empty preset no longer throws a ReferenceError.

In `@docs/pack_msx_game_fr.html`:
- Around line 859-871: Auto-fill title uses raw common prefix from
getCommonPrefix(diskNames...) which leaves trailing disk markers like "Game Disk
" causing titles like "Game Disk"; before calling cleanTitle() strip trailing
disk/side markers (e.g. patterns like /\s*(disk|side)\s*\d+$/i or common
trailing markers such as /(?:\s+disk|\s+side)\s*\d+$/i) from the prefix computed
from getCommonPrefix; update the logic around prefix/title assignment (the
variables/functions: getCommonPrefix, cleanTitle, titleInput, diskNames,
fileArray) to trim that trailing marker, then call cleanTitle(prefixTrimmed ||
fileArray[0].name) and set titleInput.value and outputInput.value as before.

In `@packers/msx/pack_msx_game.py`:
- Line 158: The bundled-engine path uses os.path.join(script_dir, '..', 'docs',
'data', 'msx', 'wmsx.js') which only goes up one directory from script_dir
(inside packers/msx) and thus resolves to packers/docs/...; update both
occurrences (the path construction using script_dir at lines defining the
bundled engine list) to go up two directories (use '..', '..', 'docs', 'data',
'msx', 'wmsx.js') so it resolves to the repo-level docs/data/msx/wmsx.js; adjust
the same change for the second occurrence referenced in the file.
- Around line 249-250: Several print calls in pack_msx_game.py use f-strings
with no placeholders (e.g., the literal calls printing "❌ WebMSX engine
(wmsx.js) not found!" and "   Searched in:" and the other constant-only messages
flagged by Ruff F541); remove the leading f from those print literals so they
become normal string literals. Search for the exact print messages shown in the
diff (and the other occurrences listed in the review) and replace print(f"...")
with print("...") for each constant-only statement to eliminate the F541
violations.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bbf8aecb-c468-4683-adde-d57faded863e

📥 Commits

Reviewing files that changed from the base of the PR and between adbe7d5 and 8159d7c.

📒 Files selected for processing (4)
  • docs/data/msx/wmsx.js
  • docs/pack_msx_game_en.html
  • docs/pack_msx_game_fr.html
  • packers/msx/pack_msx_game.py

Comment on lines +855 to +856
// Sort by name
files.sort(function(a, b) { return a.name.localeCompare(b.name); });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n docs/pack_msx_game_en.html | sed -n '850,860p'

Repository: aciderix/portable-retro-games

Length of output: 468


🏁 Script executed:

node << 'EOF'
// Test localeCompare behavior
const names = ["Disk 10.dsk", "Disk 2.dsk", "Disk 1.dsk", "Disk 20.dsk"];

console.log("=== Without numeric option ===");
const sorted1 = [...names].sort((a, b) => a.localeCompare(b.name));
console.log("Sorted:", sorted1);

console.log("\n=== With numeric: true ===");
const sorted2 = [...names].sort((a, b) => a.localeCompare(b, undefined, { numeric: true }));
console.log("Sorted:", sorted2);

console.log("\n=== With numeric: true, sensitivity: 'base' ===");
const sorted3 = [...names].sort((a, b) => a.localeCompare(b, undefined, { numeric: true, sensitivity: 'base' }));
console.log("Sorted:", sorted3);

// Also test the comparison direction
console.log("\n=== Direct comparison ===");
console.log("'Disk 10.dsk'.localeCompare('Disk 2.dsk'):", "Disk 10.dsk".localeCompare("Disk 2.dsk"));
console.log("'Disk 10.dsk'.localeCompare('Disk 2.dsk', undefined, {numeric: true}):", "Disk 10.dsk".localeCompare("Disk 2.dsk", undefined, {numeric: true}));
EOF

Repository: aciderix/portable-retro-games

Length of output: 533


🏁 Script executed:

grep -n "DISK_IMAGES\|selectedFiles" docs/pack_msx_game_en.html | head -20

Repository: aciderix/portable-retro-games

Length of output: 1106


Use numeric string collation for multi-disk filename sorting.

Plain localeCompare() sorts Disk 10.dsk before Disk 2.dsk, causing incorrect disk order. Since DISK_IMAGES[0] (line 1087) is the boot disk, this will generate packs that boot with the wrong disk first.

🔧 Proposed fix
-    files.sort(function(a, b) { return a.name.localeCompare(b.name); });
+    files.sort(function(a, b) {
+        return a.name.localeCompare(b.name, undefined, { numeric: true, sensitivity: 'base' });
+    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/pack_msx_game_en.html` around lines 855 - 856, The current sort using
files.sort(function(a, b) { return a.name.localeCompare(b.name); }) incorrectly
orders numeric filenames (e.g., "Disk 10.dsk" before "Disk 2.dsk"); change the
comparison to use numeric-aware collation (e.g., localeCompare with the numeric
option or an Intl.Collator configured with {numeric: true, sensitivity: 'base'})
so filenames sort naturally and ensure DISK_IMAGES[0] remains the boot disk.
Locate the files.sort call and replace the comparator with a numeric-aware
localeCompare or Intl.Collator comparator applied to a.name and b.name.

'<script>\n' +
'WMSX.' + mediaParam + ' = "data:application/octet-stream;base64,' + base64Data + '";\n' +
'WMSX.MACHINE = "' + machineType + '";\n' +
(soundPreset && SOUND_PRESETS[soundPreset] ? 'WMSX.PRESETS = "' + SOUND_PRESETS[soundPreset] + '";\n' : '') +
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, check the English file around the mentioned lines (1007, 1088)
echo "=== Checking English file around line 1007 ===" 
sed -n '1000,1015p' docs/pack_msx_game_en.html

echo ""
echo "=== Checking English file around line 1088 ===" 
sed -n '1080,1095p' docs/pack_msx_game_en.html

Repository: aciderix/portable-retro-games

Length of output: 1510


🏁 Script executed:

# Check if SOUND_PRESETS is declared anywhere in English file
echo "=== Searching for SOUND_PRESETS in English file ===" 
rg 'SOUND_PRESETS' docs/pack_msx_game_en.html

Repository: aciderix/portable-retro-games

Length of output: 360


🏁 Script executed:

# Check French file around lines 674-680
echo "=== Checking French file around line 674-680 ===" 
sed -n '670,685p' docs/pack_msx_game_fr.html

Repository: aciderix/portable-retro-games

Length of output: 559


🏁 Script executed:

# Check if SOUND_PRESETS is declared in French file
echo "=== Searching for SOUND_PRESETS in French file ===" 
rg 'SOUND_PRESETS' docs/pack_msx_game_fr.html

Repository: aciderix/portable-retro-games

Length of output: 629


Declare SOUND_PRESETS before using the sound selector.

Lines 1007 and 1088 reference SOUND_PRESETS[soundPreset], but this object is never declared in the English file. Selecting any non-empty sound preset will throw a ReferenceError and abort HTML generation. The French page defines this object at docs/pack_msx_game_fr.html:674-680.

🔧 Proposed fix
 var FILE_TYPE_MAP = {
     '.rom': { param: 'CARTRIDGE1_URL', label: 'ROM Cartridge', defaultMachine: 'MSX2P' },
     '.mx1': { param: 'CARTRIDGE1_URL', label: 'MX1 Cartridge', defaultMachine: 'MSX2P' },
     '.mx2': { param: 'CARTRIDGE1_URL', label: 'MX2 Cartridge', defaultMachine: 'MSX2' },
     '.dsk': { param: 'DISKA_URL', label: 'Floppy Disk', defaultMachine: 'MSX2P' },
     '.cas': { param: 'TAPE_URL', label: 'Cassette Tape', defaultMachine: 'MSX1' },
     '.zip': { param: 'AUTODETECT_URL', label: 'ZIP Archive (auto-detect)', defaultMachine: 'MSX2P' },
 };
+
+var SOUND_PRESETS = {
+    'SCCI': 'SCCI',
+    'SCC': 'SCC',
+    'FMPAC': 'FMPAC',
+    'MSXMUSIC': 'MSXMUSIC',
+    'MEGARAM': 'MEGARAM',
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
(soundPreset && SOUND_PRESETS[soundPreset] ? 'WMSX.PRESETS = "' + SOUND_PRESETS[soundPreset] + '";\n' : '') +
var FILE_TYPE_MAP = {
'.rom': { param: 'CARTRIDGE1_URL', label: 'ROM Cartridge', defaultMachine: 'MSX2P' },
'.mx1': { param: 'CARTRIDGE1_URL', label: 'MX1 Cartridge', defaultMachine: 'MSX2P' },
'.mx2': { param: 'CARTRIDGE1_URL', label: 'MX2 Cartridge', defaultMachine: 'MSX2' },
'.dsk': { param: 'DISKA_URL', label: 'Floppy Disk', defaultMachine: 'MSX2P' },
'.cas': { param: 'TAPE_URL', label: 'Cassette Tape', defaultMachine: 'MSX1' },
'.zip': { param: 'AUTODETECT_URL', label: 'ZIP Archive (auto-detect)', defaultMachine: 'MSX2P' },
};
var SOUND_PRESETS = {
'SCCI': 'SCCI',
'SCC': 'SCC',
'FMPAC': 'FMPAC',
'MSXMUSIC': 'MSXMUSIC',
'MEGARAM': 'MEGARAM',
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/pack_msx_game_en.html` at line 1007, SOUND_PRESETS is referenced by the
sound selector expression (soundPreset and SOUND_PRESETS[soundPreset]) but never
declared in this English HTML; add a declaration for the SOUND_PRESETS object
(matching the structure used in the French file) before any usage so
SOUND_PRESETS[soundPreset] resolves correctly—ensure the declaration appears
prior to the code that builds WMSX.PRESETS and mirrors the entries/formats from
docs/pack_msx_game_fr.html so selecting a non-empty preset no longer throws a
ReferenceError.

Comment on lines +859 to +871
// Auto-fill title from common prefix
var prefix = getCommonPrefix(diskNames.map(function(n) {
var dotIdx = n.lastIndexOf('.');
return dotIdx !== -1 ? n.substring(0, dotIdx) : n;
}));
var title = cleanTitle(prefix || fileArray[0].name);
titleInput.value = title;

// Auto-set machine
machineSelect.value = detectedType.defaultMachine;

// Auto-fill output filename
outputInput.value = (title || 'multidisk').toLowerCase().replace(/\s+/g, '_') + '.html';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Trim the trailing disk marker before auto-filling the title.

For common sets like Game Disk 1.dsk / Game Disk 2.dsk, getCommonPrefix() returns Game Disk , so the generated title and default filename become Game Disk instead of Game. The English page already strips that suffix before calling cleanTitle().

🔧 Proposed fix
         var prefix = getCommonPrefix(diskNames.map(function(n) {
             var dotIdx = n.lastIndexOf('.');
             return dotIdx !== -1 ? n.substring(0, dotIdx) : n;
         }));
+        prefix = prefix.replace(/[\s_\-\.]*(?:dis[ck]|disque)?[\s_\-\.]*\d*[\s_\-\.]*$/i, '');
         var title = cleanTitle(prefix || fileArray[0].name);
         titleInput.value = title;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Auto-fill title from common prefix
var prefix = getCommonPrefix(diskNames.map(function(n) {
var dotIdx = n.lastIndexOf('.');
return dotIdx !== -1 ? n.substring(0, dotIdx) : n;
}));
var title = cleanTitle(prefix || fileArray[0].name);
titleInput.value = title;
// Auto-set machine
machineSelect.value = detectedType.defaultMachine;
// Auto-fill output filename
outputInput.value = (title || 'multidisk').toLowerCase().replace(/\s+/g, '_') + '.html';
// Auto-fill title from common prefix
var prefix = getCommonPrefix(diskNames.map(function(n) {
var dotIdx = n.lastIndexOf('.');
return dotIdx !== -1 ? n.substring(0, dotIdx) : n;
}));
prefix = prefix.replace(/[\s_\-\.]*(?:dis[ck]|disque)?[\s_\-\.]*\d*[\s_\-\.]*$/i, '');
var title = cleanTitle(prefix || fileArray[0].name);
titleInput.value = title;
// Auto-set machine
machineSelect.value = detectedType.defaultMachine;
// Auto-fill output filename
outputInput.value = (title || 'multidisk').toLowerCase().replace(/\s+/g, '_') + '.html';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/pack_msx_game_fr.html` around lines 859 - 871, Auto-fill title uses raw
common prefix from getCommonPrefix(diskNames...) which leaves trailing disk
markers like "Game Disk " causing titles like "Game Disk"; before calling
cleanTitle() strip trailing disk/side markers (e.g. patterns like
/\s*(disk|side)\s*\d+$/i or common trailing markers such as
/(?:\s+disk|\s+side)\s*\d+$/i) from the prefix computed from getCommonPrefix;
update the logic around prefix/title assignment (the variables/functions:
getCommonPrefix, cleanTitle, titleInput, diskNames, fileArray) to trim that
trailing marker, then call cleanTitle(prefixTrimmed || fileArray[0].name) and
set titleInput.value and outputInput.value as before.


candidates += [
os.path.join(script_dir, 'wmsx.js'),
os.path.join(script_dir, '..', 'docs', 'data', 'msx', 'wmsx.js'),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix the bundled-engine path.

Line 158 and Line 253 only go up one directory from packers/msx, so they resolve to packers/docs/data/msx/wmsx.js instead of the repo-level docs/data/msx/wmsx.js. Invoking the CLI outside the repo root will miss the checked-in engine even though it exists.

🔧 Proposed fix
-        os.path.join(script_dir, '..', 'docs', 'data', 'msx', 'wmsx.js'),
+        os.path.join(script_dir, '..', '..', 'docs', 'data', 'msx', 'wmsx.js'),
...
-        print(f"     • {os.path.abspath(os.path.join(script_dir, '..', 'docs', 'data', 'msx', 'wmsx.js'))}")
+        print(f"     • {os.path.abspath(os.path.join(script_dir, '..', '..', 'docs', 'data', 'msx', 'wmsx.js'))}")

Also applies to: 253-253

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packers/msx/pack_msx_game.py` at line 158, The bundled-engine path uses
os.path.join(script_dir, '..', 'docs', 'data', 'msx', 'wmsx.js') which only goes
up one directory from script_dir (inside packers/msx) and thus resolves to
packers/docs/...; update both occurrences (the path construction using
script_dir at lines defining the bundled engine list) to go up two directories
(use '..', '..', 'docs', 'data', 'msx', 'wmsx.js') so it resolves to the
repo-level docs/data/msx/wmsx.js; adjust the same change for the second
occurrence referenced in the file.

Comment on lines +249 to +250
print(f"❌ WebMSX engine (wmsx.js) not found!")
print(f" Searched in:")
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

git ls-files | grep -E "pack_msx_game\.py|packers.*msx"

Repository: aciderix/portable-retro-games

Length of output: 101


🏁 Script executed:

head -500 packers/msx/pack_msx_game.py | tail -300 | cat -n

Repository: aciderix/portable-retro-games

Length of output: 12740


🏁 Script executed:

sed -n '240,330p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 3822


🏁 Script executed:

wc -l packers/msx/pack_msx_game.py

Repository: aciderix/portable-retro-games

Length of output: 105


🏁 Script executed:

sed -n '250,300p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 2306


🏁 Script executed:

sed -n '318,330p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 640


🏁 Script executed:

sed -n '360,375p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 801


🏁 Script executed:

sed -n '395,460p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 2791


🏁 Script executed:

sed -n '254,262p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 425


🏁 Script executed:

sed -n '289,296p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 352


🏁 Script executed:

sed -n '320,328p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 502


🏁 Script executed:

sed -n '365,372p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 352


🏁 Script executed:

sed -n '450,460p' packers/msx/pack_msx_game.py | cat -n

Repository: aciderix/portable-retro-games

Length of output: 504


🏁 Script executed:

sed -n '320,330p' packers/msx/pack_msx_game.py

Repository: aciderix/portable-retro-games

Length of output: 476


🏁 Script executed:

sed -n '449,460p' packers/msx/pack_msx_game.py

Repository: aciderix/portable-retro-games

Length of output: 490


🏁 Script executed:

python -m pip install ruff -q 2>/dev/null && python -m ruff check packers/msx/pack_msx_game.py --select F541 2>/dev/null || echo "Checking with ruff..."

Repository: aciderix/portable-retro-games

Length of output: 9218


Remove unnecessary f prefix from constant-only print statements.

Lines 249, 250, 256, 261, 273, 281, 293, 322, 323, 363, 364, 369, 399, 423, 454, and 455 contain f-string literals with no placeholders. Ruff flags these as F541 violations.

Proposed fixes
-        print(f"❌ WebMSX engine (wmsx.js) not found!")
-        print(f"   Searched in:")
+        print("❌ WebMSX engine (wmsx.js) not found!")
+        print("   Searched in:")
-        print(f"   Use --engine <path> to specify the location of wmsx.js")
+        print("   Use --engine <path> to specify the location of wmsx.js")
-    print(f"  🎮 MSX Game Packer")
+    print("  🎮 MSX Game Packer")
-    print(f"📦 Reading game file...")
+    print("📦 Reading game file...")
-    print(f"📦 Reading WebMSX engine...")
+    print("📦 Reading WebMSX engine...")
-    print(f"\n🏗️  Generating HTML...")
+    print("\n🏗️  Generating HTML...")
-    print(f"  🔌 100% offline — no internet needed")
-    print(f"  🌐 Open in any modern browser")
+    print("  🔌 100% offline — no internet needed")
+    print("  🌐 Open in any modern browser")
-    print(f"  🎮 MSX Multi-Disk Packer")
+    print("  🎮 MSX Multi-Disk Packer")
-    print(f"\n🏗️  Generating multi-disk HTML...")
+    print("\n🏗️  Generating multi-disk HTML...")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
print(f"❌ WebMSX engine (wmsx.js) not found!")
print(f" Searched in:")
print("❌ WebMSX engine (wmsx.js) not found!")
print(" Searched in:")
🧰 Tools
🪛 Ruff (0.15.4)

[error] 249-249: f-string without any placeholders

Remove extraneous f prefix

(F541)


[error] 250-250: f-string without any placeholders

Remove extraneous f prefix

(F541)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packers/msx/pack_msx_game.py` around lines 249 - 250, Several print calls in
pack_msx_game.py use f-strings with no placeholders (e.g., the literal calls
printing "❌ WebMSX engine (wmsx.js) not found!" and "   Searched in:" and the
other constant-only messages flagged by Ruff F541); remove the leading f from
those print literals so they become normal string literals. Search for the exact
print messages shown in the diff (and the other occurrences listed in the
review) and replace print(f"...") with print("...") for each constant-only
statement to eliminate the F541 violations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant