Skip to content

fix: role_history records the old role instead of the new one - #1421

Closed
Karlious wants to merge 1 commit into
Adeptus-Dominus:mainfrom
Karlious:fix/role-history-old-role
Closed

fix: role_history records the old role instead of the new one#1421
Karlious wants to merge 1 commit into
Adeptus-Dominus:mainfrom
Karlious:fix/role-history-old-role

Conversation

@Karlious

@Karlious Karlious commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

What's wrong

In update_role() (scripts/scr_marine_struct/scr_marine_struct.gml), the role_history entry is pushed after the role has already been overwritten:

obj_ini.role[company][marine_number] = new_role;
if (instance_exists(obj_controller)) {
    array_push(role_history, [role(), obj_controller.turn]);
}

role() is a live read (return obj_ini.role[company][marine_number];), not a cached value, so by the time it's called here it returns new_role, not the role the marine actually held before the change.

Why this is unintended

Every other place in the same file that records a role into history captures it before overwriting the field — see the struct-init logic a few lines up (role_history = [[role, obj_controller.turn]] style, built from the role at creation time, not a post-write read). The ordering here is a straightforward "the value was already overwritten before it was logged" bug, not a deliberate choice — nothing downstream reads role_history expecting the new role, and there's no comment or intent suggesting this was on purpose.

Before / after

  • Before: every promotion/role change appends [new_role, turn] to role_history — the marine's logged career history shows the role he was promoted to at each step, never the role he actually held. The record is off by one forever (and the very first entry a marine ever gets is meaningless, since it just repeats itself).
  • After: role_history correctly logs [old_role, turn] — the role the marine held immediately before the change, matching what "history" is supposed to mean.

How to observe it in game

  1. Promote or reassign any marine's role a few times (e.g. Recruit -> Battle-Brother -> Sergeant).
  2. Inspect that marine's role_history (e.g. via a save file, or any UI/tooltip that surfaces it).
  3. Every entry shows the role after that step, not the role held at that point in the marine's career — e.g. the entry logged at the Recruit -> Battle-Brother transition reads "Battle-Brother", not "Recruit".

Fix

Capture role() into a local before the write, then log the local instead of re-reading the (now-updated) field.

🤖 Generated with Claude Code


Summary by cubic

Fix role history logging so it records the marine’s previous role at each change, not the new one. We now capture the old role before updating and log it with the current turn, correcting the off-by-one career history.

Written for commit 151823f. Summary will update on new commits.

Review in cubic

update_role() pushed [role(), turn] into role_history after
obj_ini.role[company][marine_number] was already set to new_role.
Since role() is a live read (obj_ini.role[company][marine_number]),
this logged the role a marine was promoted TO at every step instead
of the role he held, making the career history record off-by-one
forever. Capture the old role into a local before the write.
@github-actions github-actions Bot added Size: Tiny Type: Fix This is a fix for a bug labels Aug 3, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 1 file

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@OH296

OH296 commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

This is just claude straight out hallucinating the function logs the new role and the turn it was given and stores it in the role_history array

@Karlious

Karlious commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

You're right — with the creation-time entry recording the starting role, appending the new role at each change gives a complete timeline, and "old role" would just duplicate the previous entry. Withdrawing this one; thanks for the quick read.

@Karlious Karlious closed this Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Size: Tiny Type: Fix This is a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants