From 2cc0db7032cf2bd2993c4a178484defc396c7aae Mon Sep 17 00:00:00 2001 From: OZAWA Sakuro <10973+sakuro@users.noreply.github.com> Date: Tue, 21 Apr 2026 14:40:42 +0900 Subject: [PATCH 1/6] :sparkles: Add --strict-version flag to mod sync (#75) Install exact MOD versions from the save file instead of the latest. Deletes newer installed versions since Factorio picks the newest zip when multiple versions coexist. --- .rubocop.yml | 3 + CHANGELOG.md | 4 + completion/_factorix.bash | 2 +- completion/_factorix.fish | 1 + completion/_factorix.zsh | 1 + doc/components/cli.md | 4 +- doc/factorix.1 | 4 + lib/factorix/cli/commands/mod/sync.rb | 140 +++++++++++----- spec/factorix/cli/commands/mod/sync_spec.rb | 172 ++++++++++++++++++++ 9 files changed, 291 insertions(+), 40 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index eb4661d..03a3013 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -50,6 +50,9 @@ Metrics/ModuleLength: Exclude: - lib/factorix.rb # Cache configuration with hierarchical backend settings +Metrics/ParameterLists: + Max: 6 + Naming/PredicateMethod: Exclude: - lib/factorix/cache/file_system.rb # fetch/store/delete return boolean but are standard cache API names diff --git a/CHANGELOG.md b/CHANGELOG.md index 59d0a22..108b330 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## [Unreleased] +### Added + +- Add `--strict-version` flag to `mod sync` to install exact MOD versions from the save file (#75) + ### Changed - Raise minimum Ruby version requirement to 3.3 (#81) diff --git a/completion/_factorix.bash b/completion/_factorix.bash index 4a5b0fc..3969353 100644 --- a/completion/_factorix.bash +++ b/completion/_factorix.bash @@ -187,7 +187,7 @@ _factorix() { ;; sync) if [[ "$cur" == -* ]]; then - COMPREPLY=($(compgen -W "$global_opts $confirmable_opts -j --jobs --keep-unlisted" -- "$cur")) + COMPREPLY=($(compgen -W "$global_opts $confirmable_opts -j --jobs --keep-unlisted --strict-version" -- "$cur")) else COMPREPLY=($(compgen -f -X '!*.zip' -- "$cur")) fi diff --git a/completion/_factorix.fish b/completion/_factorix.fish index 66cf4de..0e889fa 100644 --- a/completion/_factorix.fish +++ b/completion/_factorix.fish @@ -243,6 +243,7 @@ complete -c factorix -n "__factorix_using_subcommand mod search" -l json -d 'Out complete -c factorix -n "__factorix_using_subcommand mod sync" -s y -l yes -d 'Skip confirmation prompts' complete -c factorix -n "__factorix_using_subcommand mod sync" -s j -l jobs -d 'Number of parallel downloads' -r complete -c factorix -n "__factorix_using_subcommand mod sync" -l keep-unlisted -d 'Keep MODs not listed in save file enabled' +complete -c factorix -n "__factorix_using_subcommand mod sync" -l strict-version -d 'Install exact MOD versions from save file' complete -c factorix -n "__factorix_using_subcommand mod sync" -ra '(__fish_complete_suffix .zip)' # mod changelog subcommands diff --git a/completion/_factorix.zsh b/completion/_factorix.zsh index fe5e1ad..467efb0 100644 --- a/completion/_factorix.zsh +++ b/completion/_factorix.zsh @@ -299,6 +299,7 @@ _factorix_mod() { $confirmable_opts \ '(-j --jobs)'{-j,--jobs}'[Number of parallel downloads]:jobs:' \ '--keep-unlisted[Keep MODs not listed in save file enabled]' \ + '--strict-version[Install exact MOD versions from save file]' \ '1:save file:_files -g "*.zip"' ;; changelog) diff --git a/doc/components/cli.md b/doc/components/cli.md index 6314387..2366d4f 100644 --- a/doc/components/cli.md +++ b/doc/components/cli.md @@ -422,13 +422,15 @@ Synchronize MOD states from a save file. **Options**: - `-j`, `--jobs` - Number of parallel downloads (default: 4) - `--keep-unlisted` - Keep MODs not listed in the save file enabled (default: disable them) +- `--strict-version` - Install exact MOD versions recorded in the save file (default: install latest) **Features**: - Extracts MOD information from save file - Downloads missing MODs concurrently - Enables/disables MODs to match save file state - Disables enabled MODs (including expansion MODs) not listed in the save file (unless `--keep-unlisted` is specified) -- Preserves existing MOD files when possible +- Without `--strict-version`: installs missing MODs at their latest available version, does not record or change version in mod-list.json +- With `--strict-version`: installs MODs at the exact version from the save file, records version in mod-list.json; deletes any installed version newer than the save version (Factorio picks the newest zip when multiple versions coexist) **Use case**: Set up MOD environment to match a specific save file diff --git a/doc/factorix.1 b/doc/factorix.1 index 24894ae..fa4d77b 100644 --- a/doc/factorix.1 +++ b/doc/factorix.1 @@ -195,6 +195,10 @@ Number of parallel downloads (default: 4). .TP .B \-\-keep\-unlisted Keep MODs not listed in the save file enabled. +.TP +.B \-\-strict\-version +Install exact MOD versions recorded in the save file (default: install latest). +Deletes any installed version newer than the save version. .SS factorix mod changelog add ENTRY Add an entry to MOD changelog. .TP diff --git a/lib/factorix/cli/commands/mod/sync.rb b/lib/factorix/cli/commands/mod/sync.rb index 7f49b90..f6be154 100644 --- a/lib/factorix/cli/commands/mod/sync.rb +++ b/lib/factorix/cli/commands/mod/sync.rb @@ -26,22 +26,25 @@ class Sync < Base desc "Sync MOD states and startup settings from a save file" example [ - "save.zip # Sync MOD(s) from save file", - "-j 8 save.zip # Use 8 parallel downloads", - "--keep-unlisted save.zip # Keep MOD(s) not in save file enabled" + "save.zip # Sync MOD(s) from save file", + "-j 8 save.zip # Use 8 parallel downloads", + "--keep-unlisted save.zip # Keep MOD(s) not in save file enabled", + "--strict-version save.zip # Install exact versions from save file" ] argument :save_file, required: true, desc: "Path to Factorio save file (.zip)" option :jobs, aliases: ["-j"], default: "4", desc: "Number of parallel downloads" option :keep_unlisted, type: :flag, default: false, desc: "Keep MOD(s) not listed in save file enabled" + option :strict_version, type: :flag, default: false, desc: "Install exact MOD versions from save file" # Execute the sync command # # @param save_file [String] Path to save file # @param jobs [Integer] Number of parallel downloads # @param keep_unlisted [Boolean] Whether to keep unlisted MODs enabled + # @param strict_version [Boolean] Whether to install exact versions from save file # @return [void] - def call(save_file:, jobs: "4", keep_unlisted: false, **) + def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) jobs = Integer(jobs) say "Loading save file: #{save_file}", prefix: :info save_data = SaveFile.load(Pathname(save_file)) @@ -56,24 +59,31 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) raise DirectoryNotFoundError, "MOD directory does not exist: #{runtime.mod_dir}" unless runtime.mod_dir.exist? # Plan phase (no side effects) - mods_to_install = find_mods_to_install(save_data.mods, installed_mods) - install_targets = mods_to_install.any? ? plan_installation(mods_to_install, graph, jobs) : [] + mods_to_install = find_mods_to_install(save_data.mods, installed_mods, strict_version:) + install_targets = mods_to_install.any? ? plan_installation(mods_to_install, graph, jobs, strict_version:) : [] + mods_to_delete = strict_version ? find_mods_to_delete(save_data.mods, installed_mods) : [] conflict_mods = find_conflict_mods(mod_list, save_data.mods, graph) - changes = plan_mod_list_changes(mod_list, save_data.mods) + changes = plan_mod_list_changes(mod_list, save_data.mods, strict_version:) unlisted_mods = keep_unlisted ? [] : find_unlisted_mods(mod_list, save_data.mods, conflict_mods) mod_list_changed = needs_confirmation?(install_targets, conflict_mods, changes, unlisted_mods) + has_changes = mod_list_changed || mods_to_delete.any? settings_changed = startup_settings_changed?(save_data.startup_settings) # Show combined plan and ask once - unless mod_list_changed || settings_changed + unless has_changes || settings_changed say "Nothing to change", prefix: :info return end - show_sync_plan(install_targets, conflict_mods, changes, unlisted_mods, settings_changed) + show_sync_plan(install_targets, mods_to_delete, conflict_mods, changes, unlisted_mods, settings_changed) return unless confirm?("Do you want to apply these changes?") # Execute phase + if mods_to_delete.any? + execute_deletions(mods_to_delete) + say "Deleted #{mods_to_delete.size} MOD package(s)", prefix: :success + end + if install_targets.any? execute_installation(install_targets, jobs) say "Installed #{install_targets.size} MOD(s)", prefix: :success @@ -94,12 +104,34 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) say "Sync completed successfully", prefix: :success end - private def find_mods_to_install(save_mods, installed_mods) - save_mods.reject do |mod_name, _mod_state| - next true if mod_name == "base" + private def find_mods_to_install(save_mods, installed_mods, strict_version: false) + save_mods.reject do |mod_name, mod_state| + mod = Factorix::MOD[name: mod_name] + next true if mod.base? || mod.expansion? + + if strict_version + installed_mods.any? {|i| i.mod == mod && i.version == mod_state.version } + else + installed_mods.any? {|i| i.mod == mod } + end + end + end + # Find installed MODs with a version newer than what the save file requires + # + # These must be deleted when using --strict-version because Factorio picks the + # newest available zip when multiple versions coexist in the MOD directory. + # + # @param save_mods [Hash] MODs from save file + # @param installed_mods [Array] Currently installed MODs + # @return [Array] Installed MODs with newer versions than the save requires + private def find_mods_to_delete(save_mods, installed_mods) + save_mods.flat_map do |mod_name, mod_state| mod = Factorix::MOD[name: mod_name] - installed_mods.any? {|installed| installed.mod == mod } + next [] if mod.base? || mod.expansion? + + save_version = mod_state.version + installed_mods.select {|i| i.mod == mod && i.version > save_version } end end @@ -108,10 +140,11 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) # @param mods_to_install [Hash] MODs to install # @param graph [Dependency::Graph] Current dependency graph # @param jobs [Integer] Number of parallel jobs + # @param strict_version [Boolean] Whether to fetch exact save versions # @return [Array] Installation targets with MOD info and releases - private def plan_installation(mods_to_install, graph, jobs) + private def plan_installation(mods_to_install, graph, jobs, strict_version:) presenter = Progress::Presenter.new(title: "\u{1F50D}\u{FE0E} Fetching MOD info", output: err) - target_infos = fetch_target_mod_info(mods_to_install, jobs, presenter) + target_infos = fetch_target_mod_info(mods_to_install, jobs, presenter, strict_version:) target_infos.each do |info| graph.add_uninstalled_mod(info[:mod_info], info[:release]) @@ -125,15 +158,16 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) # @param mods_to_install [Hash] MODs to install # @param jobs [Integer] Number of parallel jobs # @param presenter [Progress::Presenter] Progress presenter + # @param strict_version [Boolean] Whether to fetch exact save versions # @return [Array] Array of {mod_name:, mod_info:, release:, version:} - private def fetch_target_mod_info(mods_to_install, jobs, presenter) + private def fetch_target_mod_info(mods_to_install, jobs, presenter, strict_version:) presenter.start(total: mods_to_install.size) pool = Concurrent::FixedThreadPool.new(jobs) futures = mods_to_install.map {|mod_name, mod_state| Concurrent::Future.execute(executor: pool) do - result = fetch_single_mod_info(mod_name, mod_state.version) + result = fetch_single_mod_info(mod_name, mod_state.version, strict_version:) presenter.update result end @@ -149,26 +183,22 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) # Fetch information for a single MOD # # @param mod_name [String] MOD name - # @param version [MODVersion] Target version + # @param version [MODVersion] Version from save file (used only when strict_version is true) + # @param strict_version [Boolean] Whether to fetch exact save version or latest # @return [Hash] {mod_name:, mod_info:, release:, version:} - private def fetch_single_mod_info(mod_name, version) + private def fetch_single_mod_info(mod_name, version, strict_version:) mod_info = portal.get_mod_full(mod_name) - release = mod_info.releases.find {|r| r.version == version } + release = if strict_version + mod_info.releases.find {|r| r.version == version } + else + mod_info.latest_release || mod_info.releases.max_by(&:version) + end unless release raise MODNotOnPortalError, "Release not found for #{mod_name}@#{version}" end - {mod_name:, mod_info:, release:, version:} - end - - # Execute the installation - # - # @param targets [Array] Installation targets - # @param jobs [Integer] Number of parallel jobs - # @return [void] - private def execute_installation(targets, jobs) - download_mods(targets, jobs) + {mod_name:, mod_info:, release:, version: release.version} end # Find MODs that conflict with enabled MODs from the save file @@ -214,8 +244,9 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) # # @param mod_list [MODList] Current MOD list # @param save_mods [Hash] MODs from save file + # @param strict_version [Boolean] Whether to record exact versions in mod-list.json # @return [Array] Change entries: {mod:, action:, ...} - private def plan_mod_list_changes(mod_list, save_mods) + private def plan_mod_list_changes(mod_list, save_mods, strict_version: false) changes = [] save_mods.each do |mod_name, mod_state| @@ -223,10 +254,10 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) next if mod.base? if mod_list.exist?(mod) - changes.concat(plan_existing_mod_changes(mod_list, mod, mod_state)) + changes.concat(plan_existing_mod_changes(mod_list, mod, mod_state, strict_version:)) else - # Not in list: add it (silently if disabled) - changes << {mod:, action: :add, to_enabled: mod_state.enabled?, to_version: mod_state.version} + to_version = strict_version ? mod_state.version : nil + changes << {mod:, action: :add, to_enabled: mod_state.enabled?, to_version:} end end @@ -238,8 +269,9 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) # @param mod_list [MODList] Current MOD list # @param mod [MOD] The MOD to plan changes for # @param mod_state [MODState] MOD state from save file + # @param strict_version [Boolean] Whether to sync versions in mod-list.json # @return [Array] Change entries for this MOD - private def plan_existing_mod_changes(mod_list, mod, mod_state) + private def plan_existing_mod_changes(mod_list, mod, mod_state, strict_version: false) current_enabled = mod_list.enabled?(mod) return plan_expansion_mod_changes(mod, mod_state, current_enabled) if mod.expansion? @@ -247,11 +279,13 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) to_enabled = mod_state.enabled? to_version = mod_state.version enabled_changed = current_enabled != to_enabled - version_changed = current_version && current_version != to_version + # Only sync versions when --strict-version is given + version_changed = strict_version && current_version != to_version if enabled_changed action = to_enabled ? :enable : :disable - [{mod:, action:, from_version: current_version, to_version:, from_enabled: current_enabled}] + recorded_version = strict_version ? to_version : current_version + [{mod:, action:, from_version: current_version, to_version: recorded_version, from_enabled: current_enabled}] elsif version_changed [{mod:, action: :update, from_version: current_version, to_version:, from_enabled: current_enabled}] else @@ -295,14 +329,20 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) # Show the combined sync plan # # @param install_targets [Array] MODs to install + # @param mods_to_delete [Array] Installed MODs to delete (newer than save version) # @param conflict_mods [Array] MODs to disable due to conflicts # @param changes [Array] MOD list changes from save file # @param unlisted_mods [Array] MODs to disable as unlisted # @param settings_changed [Boolean] Whether startup settings will be updated # @return [void] - private def show_sync_plan(install_targets, conflict_mods, changes, unlisted_mods, settings_changed) + private def show_sync_plan(install_targets, mods_to_delete, conflict_mods, changes, unlisted_mods, settings_changed) say "Planning to sync MOD(s):", prefix: :info + if mods_to_delete.any? + say " Delete (newer than save version):" + mods_to_delete.each {|m| say " - #{m.mod}@#{m.version} (#{m.path.basename})" } + end + if install_targets.any? say " Install:" install_targets.each {|t| say " - #{t[:mod]}@#{t[:release].version}" } @@ -397,6 +437,30 @@ def call(save_file:, jobs: "4", keep_unlisted: false, **) end end + # Delete installed MOD packages that are newer than the save file requires + # + # @param mods_to_delete [Array] MOD packages to delete + # @return [void] + private def execute_deletions(mods_to_delete) + mods_to_delete.each do |installed_mod| + if installed_mod.form == InstalledMOD::DIRECTORY_FORM + installed_mod.path.rmtree + else + installed_mod.path.delete + end + logger.debug("Deleted MOD package", mod_name: installed_mod.mod.name, version: installed_mod.version.to_s, path: installed_mod.path.to_s) + end + end + + # Execute the installation + # + # @param targets [Array] Installation targets + # @param jobs [Integer] Number of parallel jobs + # @return [void] + private def execute_installation(targets, jobs) + download_mods(targets, jobs) + end + # Update mod-settings.dat with startup settings from save file # # @param startup_settings [MODSettings::Section] Startup settings from save file diff --git a/spec/factorix/cli/commands/mod/sync_spec.rb b/spec/factorix/cli/commands/mod/sync_spec.rb index 1e31bc4..bdb8911 100644 --- a/spec/factorix/cli/commands/mod/sync_spec.rb +++ b/spec/factorix/cli/commands/mod/sync_spec.rb @@ -213,5 +213,177 @@ expect(mod_settings).not_to have_received(:save) end end + + describe "--strict-version" do + let(:save_version) { Factorix::MODVersion.from_string("1.0.0") } + let(:newer_version) { Factorix::MODVersion.from_string("1.1.0") } + let(:older_version) { Factorix::MODVersion.from_string("0.9.0") } + + let(:newer_mod_path) { Pathname("/tmp/mods/test-mod_1.1.0.zip") } + let(:newer_installed_mod) do + Factorix::InstalledMOD[ + mod: Factorix::MOD[name: "test-mod"], + version: newer_version, + form: Factorix::InstalledMOD::ZIP_FORM, + path: newer_mod_path, + info: Factorix::InfoJSON[ + name: "test-mod", + version: newer_version, + title: "Test MOD", + author: "Test Author", + description: "Test description", + factorio_version: "2.0", + dependencies: [] + ] + ] + end + + let(:older_installed_mod) do + Factorix::InstalledMOD[ + mod: Factorix::MOD[name: "test-mod"], + version: older_version, + form: Factorix::InstalledMOD::ZIP_FORM, + path: Pathname("/tmp/mods/test-mod_0.9.0.zip"), + info: Factorix::InfoJSON[ + name: "test-mod", + version: older_version, + title: "Test MOD", + author: "Test Author", + description: "Test description", + factorio_version: "2.0", + dependencies: [] + ] + ] + end + + context "when already in sync at exact version" do + before do + mod_list.add(Factorix::MOD[name: "base"], enabled: true, version: base_mod_version) + mod_list.add(Factorix::MOD[name: "test-mod"], enabled: true, version: save_version) + end + + it "does not ask for confirmation" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(command).not_to have_received(:confirm?) + end + + it "does not save mod-list.json" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(mod_list).not_to have_received(:save) + end + end + + context "when a newer version is installed" do + before do + mod_list.add(Factorix::MOD[name: "base"], enabled: true, version: base_mod_version) + mod_list.add(Factorix::MOD[name: "test-mod"], enabled: true, version: newer_version) + allow(Factorix::InstalledMOD).to receive(:all).and_return([ + Factorix::InstalledMOD[ + mod: Factorix::MOD[name: "base"], + version: base_mod_version, + form: Factorix::InstalledMOD::DIRECTORY_FORM, + path: Pathname("/path/to/base"), + info: base_info + ], + newer_installed_mod + ]) + allow(command).to receive(:execute_deletions).and_call_original + allow(newer_mod_path).to receive(:delete) + end + + context "when also downloading the save version" do + before do + allow(command).to receive(:execute_installation) + allow(command).to receive(:plan_installation).and_return([]) + end + + it "asks for confirmation" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(command).to have_received(:confirm?).once + end + + it "deletes the newer version zip" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(newer_mod_path).to have_received(:delete) + end + + it "records exact save version in mod-list.json" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(mod_list.version(Factorix::MOD[name: "test-mod"])).to eq(save_version) + end + end + end + + context "when an older version is installed" do + before do + mod_list.add(Factorix::MOD[name: "base"], enabled: true, version: base_mod_version) + mod_list.add(Factorix::MOD[name: "test-mod"], enabled: true, version: older_version) + allow(Factorix::InstalledMOD).to receive(:all).and_return([ + Factorix::InstalledMOD[ + mod: Factorix::MOD[name: "base"], + version: base_mod_version, + form: Factorix::InstalledMOD::DIRECTORY_FORM, + path: Pathname("/path/to/base"), + info: base_info + ], + older_installed_mod + ]) + allow(command).to receive(:execute_installation) + allow(command).to receive(:plan_installation).and_return([]) + end + + it "does not delete the older version zip" do + allow(older_installed_mod.path).to receive(:delete) + + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(older_installed_mod.path).not_to have_received(:delete) + end + + it "records exact save version in mod-list.json" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(mod_list.version(Factorix::MOD[name: "test-mod"])).to eq(save_version) + end + end + + context "when user declines confirmation" do + before do + mod_list.add(Factorix::MOD[name: "base"], enabled: true, version: base_mod_version) + mod_list.add(Factorix::MOD[name: "test-mod"], enabled: true, version: newer_version) + allow(Factorix::InstalledMOD).to receive(:all).and_return([ + Factorix::InstalledMOD[ + mod: Factorix::MOD[name: "base"], + version: base_mod_version, + form: Factorix::InstalledMOD::DIRECTORY_FORM, + path: Pathname("/path/to/base"), + info: base_info + ], + newer_installed_mod + ]) + allow(command).to receive(:execute_deletions) + allow(command).to receive(:execute_installation) + allow(command).to receive_messages(confirm?: false, plan_installation: []) + allow(newer_mod_path).to receive(:delete) + end + + it "does not delete the newer version zip" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(command).not_to have_received(:execute_deletions) + end + + it "does not save mod-list.json" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(mod_list).not_to have_received(:save) + end + end + end end end From cdc1ab25c0a63da5ee18700aa4391e50ae6bbeb5 Mon Sep 17 00:00:00 2001 From: OZAWA Sakuro <10973+sakuro@users.noreply.github.com> Date: Tue, 21 Apr 2026 19:23:13 +0900 Subject: [PATCH 2/6] :bug: Include MOD name in not-found-on-portal error message The portal API response body contains "Mod not found" without the MOD name. By prioritizing api_message, the MOD name was lost from the error shown to users. --- lib/factorix/api/mod_management_api.rb | 16 ++++++++-------- lib/factorix/api/mod_portal_api.rb | 8 ++++---- spec/factorix/api/mod_management_api_spec.rb | 11 +---------- spec/factorix/api/mod_portal_api_spec.rb | 15 ++------------- 4 files changed, 15 insertions(+), 35 deletions(-) diff --git a/lib/factorix/api/mod_management_api.rb b/lib/factorix/api/mod_management_api.rb index 6018100..8adb00f 100644 --- a/lib/factorix/api/mod_management_api.rb +++ b/lib/factorix/api/mod_management_api.rb @@ -76,8 +76,8 @@ def init_upload(mod_name) response = client.post(uri, body:, headers: build_auth_header, content_type: "application/x-www-form-urlencoded") parse_upload_url(response) - rescue HTTPNotFoundError => e - raise MODNotOnPortalError, e.api_message || "MOD '#{mod_name}' not found on portal" + rescue HTTPNotFoundError + raise MODNotOnPortalError, "MOD '#{mod_name}' not found on portal" end # Complete upload (works for both publish and update scenarios) @@ -136,8 +136,8 @@ def edit_details(mod_name, **metadata) client.post(uri, body:, headers: build_auth_header, content_type: "application/x-www-form-urlencoded") logger.info("Edit completed successfully", mod: mod_name) publish("mod.changed", mod: mod_name) - rescue HTTPNotFoundError => e - raise MODNotOnPortalError, e.api_message || "MOD '#{mod_name}' not found on portal" + rescue HTTPNotFoundError + raise MODNotOnPortalError, "MOD '#{mod_name}' not found on portal" end # Initialize image upload for a MOD @@ -155,8 +155,8 @@ def init_image_upload(mod_name) response = client.post(uri, body:, headers: build_auth_header, content_type: "application/x-www-form-urlencoded") parse_upload_url(response) - rescue HTTPNotFoundError => e - raise MODNotOnPortalError, e.api_message || "MOD '#{mod_name}' not found on portal" + rescue HTTPNotFoundError + raise MODNotOnPortalError, "MOD '#{mod_name}' not found on portal" end # Complete image upload @@ -200,8 +200,8 @@ def edit_images(mod_name, image_ids) client.post(uri, body:, headers: build_auth_header, content_type: "application/x-www-form-urlencoded") logger.info("Images updated successfully", mod: mod_name) publish("mod.changed", mod: mod_name) - rescue HTTPNotFoundError => e - raise MODNotOnPortalError, e.api_message || "MOD '#{mod_name}' not found on portal" + rescue HTTPNotFoundError + raise MODNotOnPortalError, "MOD '#{mod_name}' not found on portal" end private def api_credential diff --git a/lib/factorix/api/mod_portal_api.rb b/lib/factorix/api/mod_portal_api.rb index c44108c..dcadf1a 100644 --- a/lib/factorix/api/mod_portal_api.rb +++ b/lib/factorix/api/mod_portal_api.rb @@ -56,8 +56,8 @@ def get_mod(name) encoded_name = ERB::Util.url_encode(name) uri = build_uri("/api/mods/#{encoded_name}") fetch_with_cache(uri) - rescue HTTPNotFoundError => e - raise MODNotOnPortalError, e.api_message || "MOD '#{name}' not found on portal" + rescue HTTPNotFoundError + raise MODNotOnPortalError, "MOD '#{name}' not found on portal" end # Retrieve detailed information for a specific MOD @@ -70,8 +70,8 @@ def get_mod_full(name) encoded_name = ERB::Util.url_encode(name) uri = build_uri("/api/mods/#{encoded_name}/full") fetch_with_cache(uri) - rescue HTTPNotFoundError => e - raise MODNotOnPortalError, e.api_message || "MOD '#{name}' not found on portal" + rescue HTTPNotFoundError + raise MODNotOnPortalError, "MOD '#{name}' not found on portal" end # Event handler for mod.changed event diff --git a/spec/factorix/api/mod_management_api_spec.rb b/spec/factorix/api/mod_management_api_spec.rb index 487ad8c..1c0183b 100644 --- a/spec/factorix/api/mod_management_api_spec.rb +++ b/spec/factorix/api/mod_management_api_spec.rb @@ -58,16 +58,7 @@ end context "when MOD not found" do - it "raises MODNotOnPortalError with api_message" do - error = Factorix::HTTPNotFoundError.new("404 Not Found", api_message: "Unknown Mod") - allow(client).to receive(:post).and_raise(error) - - expect { - api.init_upload("nonexistent-mod") - }.to raise_error(Factorix::MODNotOnPortalError, "Unknown Mod") - end - - it "raises MODNotOnPortalError with fallback message when api_message is nil" do + it "raises MODNotOnPortalError with MOD name" do error = Factorix::HTTPNotFoundError.new("404 Not Found") allow(client).to receive(:post).and_raise(error) diff --git a/spec/factorix/api/mod_portal_api_spec.rb b/spec/factorix/api/mod_portal_api_spec.rb index 1545bbc..4ae2467 100644 --- a/spec/factorix/api/mod_portal_api_spec.rb +++ b/spec/factorix/api/mod_portal_api_spec.rb @@ -105,24 +105,13 @@ end describe "error handling" do - context "when API returns 404 with api_message" do - before do - error = Factorix::HTTPNotFoundError.new("404 Not Found", api_message: "Mod not found") - allow(client).to receive(:get).and_raise(error) - end - - it "raises MODNotOnPortalError with api_message" do - expect { api.get_mod("nonexistent") }.to raise_error(Factorix::MODNotOnPortalError, "Mod not found") - end - end - - context "when API returns 404 without api_message" do + context "when API returns 404" do before do error = Factorix::HTTPNotFoundError.new("404 Not Found") allow(client).to receive(:get).and_raise(error) end - it "raises MODNotOnPortalError with fallback message" do + it "raises MODNotOnPortalError with MOD name" do expect { api.get_mod("nonexistent") }.to raise_error(Factorix::MODNotOnPortalError, "MOD 'nonexistent' not found on portal") end end From 6ba040b0c34e9d9ee5728ea102ffc7395c15f13d Mon Sep 17 00:00:00 2001 From: OZAWA Sakuro <10973+sakuro@users.noreply.github.com> Date: Tue, 21 Apr 2026 19:39:26 +0900 Subject: [PATCH 3/6] :sparkles: Show currently installed version in mod sync install plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When replacing an existing MOD (e.g. with --strict-version), display the transition as "mod-name (old → new)" instead of "mod-name@new". --- lib/factorix/cli/commands/mod/sync.rb | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/factorix/cli/commands/mod/sync.rb b/lib/factorix/cli/commands/mod/sync.rb index f6be154..b32b308 100644 --- a/lib/factorix/cli/commands/mod/sync.rb +++ b/lib/factorix/cli/commands/mod/sync.rb @@ -61,6 +61,7 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) # Plan phase (no side effects) mods_to_install = find_mods_to_install(save_data.mods, installed_mods, strict_version:) install_targets = mods_to_install.any? ? plan_installation(mods_to_install, graph, jobs, strict_version:) : [] + enrich_install_targets_with_current_version(install_targets, installed_mods) mods_to_delete = strict_version ? find_mods_to_delete(save_data.mods, installed_mods) : [] conflict_mods = find_conflict_mods(mod_list, save_data.mods, graph) changes = plan_mod_list_changes(mod_list, save_data.mods, strict_version:) @@ -117,6 +118,18 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) end end + # Enrich install targets with the currently installed version for display purposes + # + # @param install_targets [Array] Install targets to enrich in-place + # @param installed_mods [Array] Currently installed MODs + # @return [void] + private def enrich_install_targets_with_current_version(install_targets, installed_mods) + install_targets.each do |target| + current = installed_mods.find {|i| i.mod == target[:mod] } + target[:from_version] = current&.version + end + end + # Find installed MODs with a version newer than what the save file requires # # These must be deleted when using --strict-version because Factorio picks the @@ -345,7 +358,10 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) if install_targets.any? say " Install:" - install_targets.each {|t| say " - #{t[:mod]}@#{t[:release].version}" } + install_targets.each do |t| + label = t[:from_version] ? "#{t[:mod]} (#{t[:from_version]} \u2192 #{t[:release].version})" : "#{t[:mod]}@#{t[:release].version}" + say " - #{label}" + end end enable_changes = changes.select {|c| c[:action] == :enable } From e29de920466d41bb8f795b5f75829f5b90d4359e Mon Sep 17 00:00:00 2001 From: OZAWA Sakuro <10973+sakuro@users.noreply.github.com> Date: Tue, 21 Apr 2026 20:23:38 +0900 Subject: [PATCH 4/6] :bug: Fix spurious version updates when mod-list.json has no version recorded plan_existing_mod_changes compared nil (from mod-list.json) against the save version, always producing an :update action even when the correct version was already installed. When mod-list.json has no version recorded for a MOD, fall back to the actually installed version for the comparison. This avoids treating already-correct installations as needing an update. --- lib/factorix/cli/commands/mod/sync.rb | 25 +++++++++++++-------- spec/factorix/cli/commands/mod/sync_spec.rb | 20 +++++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/lib/factorix/cli/commands/mod/sync.rb b/lib/factorix/cli/commands/mod/sync.rb index b32b308..7f4a65e 100644 --- a/lib/factorix/cli/commands/mod/sync.rb +++ b/lib/factorix/cli/commands/mod/sync.rb @@ -64,7 +64,7 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) enrich_install_targets_with_current_version(install_targets, installed_mods) mods_to_delete = strict_version ? find_mods_to_delete(save_data.mods, installed_mods) : [] conflict_mods = find_conflict_mods(mod_list, save_data.mods, graph) - changes = plan_mod_list_changes(mod_list, save_data.mods, strict_version:) + changes = plan_mod_list_changes(mod_list, save_data.mods, installed_mods, strict_version:) unlisted_mods = keep_unlisted ? [] : find_unlisted_mods(mod_list, save_data.mods, conflict_mods) mod_list_changed = needs_confirmation?(install_targets, conflict_mods, changes, unlisted_mods) has_changes = mod_list_changed || mods_to_delete.any? @@ -257,9 +257,10 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) # # @param mod_list [MODList] Current MOD list # @param save_mods [Hash] MODs from save file + # @param installed_mods [Array] Currently installed MODs # @param strict_version [Boolean] Whether to record exact versions in mod-list.json # @return [Array] Change entries: {mod:, action:, ...} - private def plan_mod_list_changes(mod_list, save_mods, strict_version: false) + private def plan_mod_list_changes(mod_list, save_mods, installed_mods, strict_version: false) changes = [] save_mods.each do |mod_name, mod_state| @@ -267,7 +268,7 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) next if mod.base? if mod_list.exist?(mod) - changes.concat(plan_existing_mod_changes(mod_list, mod, mod_state, strict_version:)) + changes.concat(plan_existing_mod_changes(mod_list, mod, mod_state, installed_mods, strict_version:)) else to_version = strict_version ? mod_state.version : nil changes << {mod:, action: :add, to_enabled: mod_state.enabled?, to_version:} @@ -282,13 +283,17 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) # @param mod_list [MODList] Current MOD list # @param mod [MOD] The MOD to plan changes for # @param mod_state [MODState] MOD state from save file + # @param installed_mods [Array] Currently installed MODs # @param strict_version [Boolean] Whether to sync versions in mod-list.json # @return [Array] Change entries for this MOD - private def plan_existing_mod_changes(mod_list, mod, mod_state, strict_version: false) + private def plan_existing_mod_changes(mod_list, mod, mod_state, installed_mods, strict_version: false) current_enabled = mod_list.enabled?(mod) return plan_expansion_mod_changes(mod, mod_state, current_enabled) if mod.expansion? - current_version = mod_list.version(mod) + # When mod-list.json has no version recorded, fall back to the installed version. + # This avoids treating already-correct installations as needing an update. + recorded_current_version = mod_list.version(mod) + current_version = recorded_current_version || installed_mods.find {|i| i.mod == mod }&.version to_enabled = mod_state.enabled? to_version = mod_state.version enabled_changed = current_enabled != to_enabled @@ -296,9 +301,8 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) version_changed = strict_version && current_version != to_version if enabled_changed - action = to_enabled ? :enable : :disable - recorded_version = strict_version ? to_version : current_version - [{mod:, action:, from_version: current_version, to_version: recorded_version, from_enabled: current_enabled}] + apply_version = strict_version ? to_version : recorded_current_version + [{mod:, action: to_enabled ? :enable : :disable, from_version: current_version, to_version: apply_version, from_enabled: current_enabled}] elsif version_changed [{mod:, action: :update, from_version: current_version, to_version:, from_enabled: current_enabled}] else @@ -382,7 +386,10 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) update_changes = changes.select {|c| c[:action] == :update } if update_changes.any? say " Update:" - update_changes.each {|c| say " - #{c[:mod]} (#{c[:from_version]} \u2192 #{c[:to_version]})" } + update_changes.each do |c| + label = c[:from_version] && c[:from_version] != c[:to_version] ? "#{c[:mod]} (#{c[:from_version]} \u2192 #{c[:to_version]})" : "#{c[:mod]}@#{c[:to_version]}" + say " - #{label}" + end end say " Update startup settings" if settings_changed diff --git a/spec/factorix/cli/commands/mod/sync_spec.rb b/spec/factorix/cli/commands/mod/sync_spec.rb index bdb8911..f210f8a 100644 --- a/spec/factorix/cli/commands/mod/sync_spec.rb +++ b/spec/factorix/cli/commands/mod/sync_spec.rb @@ -275,6 +275,26 @@ end end + context "when mod-list.json has no version recorded but installed version matches save version" do + before do + # Version not recorded (nil) - happens after a non-strict sync + mod_list.add(Factorix::MOD[name: "base"], enabled: true, version: base_mod_version) + mod_list.add(Factorix::MOD[name: "test-mod"], enabled: true) + end + + it "does not ask for confirmation" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(command).not_to have_received(:confirm?) + end + + it "does not save mod-list.json" do + run_command(command, %W[--strict-version #{save_file_path}]) + + expect(mod_list).not_to have_received(:save) + end + end + context "when a newer version is installed" do before do mod_list.add(Factorix::MOD[name: "base"], enabled: true, version: base_mod_version) From 85eeed0548f9fc241e6aa03ca9d5aab1d29363d2 Mon Sep 17 00:00:00 2001 From: OZAWA Sakuro <10973+sakuro@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:08:18 +0900 Subject: [PATCH 5/6] :bug: Show error message when operation is blocked by running game RequiresGameStopped is prepended after CommandWrapper, so it sits outer in the call chain. When the game is running, the exception is raised before super is called, meaning CommandWrapper's rescue block is never entered and no error message is displayed. Display the error via say before raising so the user sees it regardless of the call chain order. --- lib/factorix/cli/commands/requires_game_stopped.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/factorix/cli/commands/requires_game_stopped.rb b/lib/factorix/cli/commands/requires_game_stopped.rb index e586ea7..2a752be 100644 --- a/lib/factorix/cli/commands/requires_game_stopped.rb +++ b/lib/factorix/cli/commands/requires_game_stopped.rb @@ -16,6 +16,11 @@ module RequiresGameStopped # @param options [Hash] command options passed to the original call method # @return [void] def call(**options) + # Set @quiet before check_game_stopped because this module is prepended + # after CommandWrapper (which normally sets @quiet), making it outer in + # the call chain. If the game is running, the exception escapes before + # CommandWrapper's rescue block is ever entered. + @quiet = options[:quiet] check_game_stopped super end @@ -23,8 +28,12 @@ def call(**options) private def check_game_stopped return unless runtime.running? + message = "Cannot perform this operation while Factorio is running. Please stop the game and try again." logger.error("Operation blocked: game is running") - raise GameRunningError, "Cannot perform this operation while Factorio is running. Please stop the game and try again." + # CommandWrapper's rescue would normally display this, but it is never + # reached when the exception is raised here (see call above). + say "Error: #{message}", prefix: :error + raise GameRunningError, message end end end From b4a8f83da380ac9df3e1a9aff54f7aaf369d8a9c Mon Sep 17 00:00:00 2001 From: OZAWA Sakuro <10973+sakuro@users.noreply.github.com> Date: Tue, 21 Apr 2026 21:20:28 +0900 Subject: [PATCH 6/6] :lipstick: Consolidate delete+install into Downgrade section in sync plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When --strict-version removes a newer zip and downloads the save version, the same MOD appeared in Delete, Install, and Update — three redundant lines. Detect mods present in both mods_to_delete and install_targets, show them once under Downgrade, and omit them from the individual sections. --- lib/factorix/cli/commands/mod/sync.rb | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/lib/factorix/cli/commands/mod/sync.rb b/lib/factorix/cli/commands/mod/sync.rb index 7f4a65e..15e8a28 100644 --- a/lib/factorix/cli/commands/mod/sync.rb +++ b/lib/factorix/cli/commands/mod/sync.rb @@ -355,14 +355,28 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) private def show_sync_plan(install_targets, mods_to_delete, conflict_mods, changes, unlisted_mods, settings_changed) say "Planning to sync MOD(s):", prefix: :info - if mods_to_delete.any? + # Mods appearing in both delete and install are downgrades (newer zip removed, + # save version downloaded). Show them once instead of in three separate sections. + downgrade_mod_set = Set.new(mods_to_delete.map(&:mod) & install_targets.map {|t| t[:mod] }) + + downgrade_targets = install_targets.select {|t| downgrade_mod_set.include?(t[:mod]) } + if downgrade_targets.any? + say " Downgrade:" + downgrade_targets.each do |t| + say " - #{t[:mod]} (#{t[:from_version]} \u2192 #{t[:release].version})" + end + end + + remaining_deletes = mods_to_delete.reject {|m| downgrade_mod_set.include?(m.mod) } + if remaining_deletes.any? say " Delete (newer than save version):" - mods_to_delete.each {|m| say " - #{m.mod}@#{m.version} (#{m.path.basename})" } + remaining_deletes.each {|m| say " - #{m.mod}@#{m.version} (#{m.path.basename})" } end - if install_targets.any? + remaining_installs = install_targets.reject {|t| downgrade_mod_set.include?(t[:mod]) } + if remaining_installs.any? say " Install:" - install_targets.each do |t| + remaining_installs.each do |t| label = t[:from_version] ? "#{t[:mod]} (#{t[:from_version]} \u2192 #{t[:release].version})" : "#{t[:mod]}@#{t[:release].version}" say " - #{label}" end @@ -383,7 +397,7 @@ def call(save_file:, jobs: "4", keep_unlisted: false, strict_version: false, **) all_disables.each {|d| say " - #{d[:mod]} #{d[:reason]}" } end - update_changes = changes.select {|c| c[:action] == :update } + update_changes = changes.select {|c| c[:action] == :update && !downgrade_mod_set.include?(c[:mod]) } if update_changes.any? say " Update:" update_changes.each do |c|