Skip to content

fix: mark skins applied only after pack files exist - #30

Merged
Drefvelin merged 1 commit into
mainfrom
fix/applied-ack-requires-pack-files
Sep 27, 2026
Merged

Drefvelin merged 1 commit into
mainfrom
fix/applied-ack-requires-pack-files

Conversation

@Drefvelin

@Drefvelin Drefvelin commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Stop /armourshop reload, pack sync, and submission delete from copying every approved-but-unwritten skin into the pending-reload queue.
  • Ack a skin as applied only after this flush's iazip, and only when its ItemsAdder config is on disk. Missing configs stay approved so the next pack pull can write them.
  • Covers the case where a reload or delete queued a skin, then the next restart zipped the old pack and told the website the skin was live.

Test plan

  • CI build on this pull request succeeds (DEV jar)
  • DEV jar enables on TFMCDev
  • ApplyAckPlannerTest passes in CI
  • Confirm /armourshop reload does not log a pending-queue import of unwritten approvals

Summary by CodeRabbit

  • Pack Updates
    • Pending reloads now retain only submissions that are still approved; newly approved submissions are not added automatically and must be written with /armourshop pack pull.
    • A submission is marked applied only after its pack files are present and the reload flush completes. Entries with missing files remain pending.
  • Reliability
    • If approval information cannot be retrieved, existing pending reloads are preserved rather than replaced.

Reload and submission delete copied every approved id into the pending
queue, and the next pack zip acked them. The website then showed those
skins as live with no ItemsAdder config on disk.

Co-authored-by: Cursor <[email protected]>
@coderabbitai

coderabbitai Bot commented Sep 27, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 557566c7-a2bd-4df4-9548-5978475d587a

📥 Commits

Reviewing files that changed from the base of the PR and between a67ae72 and 7a7c179.

📒 Files selected for processing (8)
  • src/main/java/net/tfminecraft/armourshop/ArmourShop.java
  • src/main/java/net/tfminecraft/armourshop/managers/CommandManager.java
  • src/main/java/net/tfminecraft/armourshop/pack/delete/SkinDeleteRunner.java
  • src/main/java/net/tfminecraft/armourshop/pack/delete/SubmissionDeleteRunner.java
  • src/main/java/net/tfminecraft/armourshop/pack/reload/ApplyAckPlanner.java
  • src/main/java/net/tfminecraft/armourshop/pack/reload/DeferredIaReloadService.java
  • src/main/java/net/tfminecraft/armourshop/pack/reload/PendingReloadQueue.java
  • src/test/java/net/tfminecraft/armourshop/pack/reload/ApplyAckPlannerTest.java
 _______________________________________
< Goodbye, code review procrastination. >
 ---------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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

@Drefvelin
Drefvelin merged commit 05f3565 into main Sep 27, 2026
1 of 2 checks passed
@Drefvelin
Drefvelin deleted the fix/applied-ack-requires-pack-files branch September 27, 2026 16:34
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