Skip to content

Fix explosion position: cross-array copy in stelem.i1#230

Merged
jonathanpeppers merged 3 commits intomainfrom
copilot/fix-explosion-position-issue
Mar 16, 2026
Merged

Fix explosion position: cross-array copy in stelem.i1#230
jonathanpeppers merged 3 commits intomainfrom
copilot/fix-explosion-position-issue

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 15, 2026

HandleStelemI1 had no codegen path for cross-array element copy (arr1[i] = arr2[j]). In shoot2's collision code, exp_x[xi] = enemy_x[ei] emitted no LDA — the accumulator held garbage from the preceding branch check, so explosions rendered at wrong positions.

Changes

  • Track source index local in the ldelem_u1 scan — capture the non-array ldloc immediately before ldelem_u1 as sourceIndex1Idx
  • Add cross-array copy handler between the self-referencing update and scalar-local cases: when sourceArray != targetArray and we have a source index, emit LDX srcIdx; LDA srcArr,X before the existing target store code

Before (no value load):

LDX $0383      ; X = xi
STA $036B,X    ; exp_x[xi] = A (garbage)

After:

LDX $0382      ; X = ei
LDA $033B,X    ; A = enemy_x[ei]
LDX $0383      ; X = xi
STA $036B,X    ; exp_x[xi] = enemy_x[ei]
Original prompt

This section details on the original issue you should resolve

<issue_title>shoot2: Explosions appear at wrong position when enemies are destroyed</issue_title>
<issue_description>In the shoot2 sample, when a bullet hits an enemy, the explosion animation doesn't appear at the enemy's position. The explosion sprites render at the wrong location on screen.

Steps to reproduce

  1. cd samples/shoot2 && dotnet run
  2. Move the ship and shoot at enemies
  3. When a bullet hits an enemy, observe the explosion — it appears at a different position than where the enemy was

Expected behavior

The explosion animation should appear centered on the enemy that was destroyed.

Likely cause

The collision detection uses branchless inline distance checks that store results in transpiler TEMP registers. By the time the explosion position is assigned (explosion_x[xi] = enemy_x[ei]), the values may have been read through array indexing that clobbers TEMP, or the local variable addresses for explosion coordinates may be incorrect.

The collision code (around line 350 in Program.cs) does:

byte dxc = (byte)((byte)(bullet_x[bi] - enemy_x[ei]) + (HIT_DISTANCE - 1));
if (dxc < (2 * HIT_DISTANCE - 1))
{
    byte dyc = (byte)((byte)(bullet_y[bi] - enemy_y[ei]) + (HIT_DISTANCE - 1));
    if (dyc < (2 * HIT_DISTANCE - 1))
    {
        // Start explosion
        for (byte xi = 0; xi < MAX_EXPLOSIONS; xi++)
        {
            if (explosion_timer[xi] == 0)
            {
                explosion_x[xi] = enemy_x[ei];
                explosion_y[xi] = enemy_y[ei];

The enemy_x[ei] and enemy_y[ei] reads inside the explosion setup may produce wrong values because the transpiler's TEMP register was overwritten by the preceding array subtractions in the distance checks.

Context

Discovered during manual testing of PR #202. The enemy spawning and collision detection work correctly (enemies are destroyed, score increments), but the visual feedback is misplaced.
</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

…positioning

The transpiler's HandleStelemI1 didn't handle the pattern arr1[i] = arr2[j]
(cross-array copy with different indices). For shoot2's explosion code
(exp_x[xi] = enemy_x[ei]), no LDA was emitted so the stored value was
whatever was left in A from a previous operation.

Added source index tracking (sourceIndex1Idx) in the ldelem scan and a new
pattern handler that emits: LDX srcIdx; LDA srcArr,X; (then existing store
code handles LDX dstIdx; STA dstArr,X).

Co-authored-by: jonathanpeppers <840039+jonathanpeppers@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix explosions appearing at wrong position for destroyed enemies Fix cross-array copy pattern in transpiler (explosion position bug) Mar 15, 2026
Copilot AI requested a review from jonathanpeppers March 15, 2026 13:52
Copy link
Copy Markdown
Owner

@jonathanpeppers jonathanpeppers left a comment

Choose a reason for hiding this comment

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

🤖 Reviewed and verified. Approving this PR.

What I checked

Code review:

  • The sourceIndex1Idx scanning in HandleStelemI1 correctly identifies the source array index from the ldloc immediately before ldelem_u1
  • The cross-array copy emission block (lines 1111-1134) correctly emits LDX source_index; LDA source_array,X then falls through to the shared target store code that emits LDX target_index; STA target_array,X
  • The condition ordering is correct — the cross-array handler (line 1111) only triggers when sourceArray1Idx != targetArrayLocalIdx, so it won't conflict with the same-array self-update handler (line 1084)

6502 disassembly verification:
After a full rebuild, I disassembled the explosion assignment area and confirmed the correct cross-array copy pattern:

$8928: LDX $0382      ; X = ei (enemy index)
$892B: LDA $033B,X    ; A = enemy_x[ei]
$892E: LDX $0383      ; X = xi (explosion index)
$8931: STA $036B,X    ; exp_x[xi] = A  ✓

$8934: LDX $0382      ; X = ei
$8937: LDA $0341,X    ; A = enemy_y[ei]
$893A: LDX $0383      ; X = xi
$893D: STA $036E,X    ; exp_y[xi] = A  ✓

Tests: All 498 tests pass (64 analyzer + 434 transpiler).

The verified.bin update is expected — the old codegen was storing 0 (the timer check result) into exp_x/exp_y instead of loading from the enemy arrays.

@jonathanpeppers jonathanpeppers marked this pull request as ready for review March 15, 2026 14:06
@jonathanpeppers jonathanpeppers changed the title Fix cross-array copy pattern in transpiler (explosion position bug) Fix explosion position: cross-array copy in stelem.i1 Mar 15, 2026
Copilot AI review requested due to automatic review settings March 15, 2026 14:06
@jonathanpeppers
Copy link
Copy Markdown
Owner

🤖 CI is green. Ready for human review.

Verified the fix by disassembling the explosion assignment area — the cross-array copy now correctly loads enemy_x[ei]/enemy_y[ei] before storing to exp_x[xi]/exp_y[xi]. Previously, it was storing 0 (the stale value in A from the exp_timer check).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a transpiler codegen gap in stelem.i1 so assignments like arr1[i] = arr2[j] correctly load the source element before storing, resolving the wrong-position explosion bug in the shoot2 sample ROM output.

Changes:

  • Extend HandleStelemI1’s ldelem_u1 scan to capture the source index local (sourceIndex1Idx).
  • Add a cross-array element-copy emission path that loads A from the source array using the source index before the existing target store logic runs.
  • Update the shoot2 verified ROM snapshot to reflect the corrected codegen.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/dotnes.tasks/Utilities/IL2NESWriter.ArrayHandling.cs Adds source-index tracking and a new cross-array stelem.i1 codegen path to load A from the source array before storing.
src/dotnes.tests/TranspilerTests.Write.shoot2.verified.bin Updates the verified binary snapshot for shoot2 to match the new correct ROM output.

Comment thread src/dotnes.tasks/Utilities/IL2NESWriter.ArrayHandling.cs Outdated
Only set sourceIndex1Idx when Locals.TryGetValue succeeds and the local
is a scalar (ArraySize == 0). Previously, the condition also accepted
untracked locals (!TryGetValue), which would throw KeyNotFoundException
when the cross-array handler later accessed Locals[sourceIndex1Idx].

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers merged commit 64bae02 into main Mar 16, 2026
1 check passed
@jonathanpeppers jonathanpeppers deleted the copilot/fix-explosion-position-issue branch March 16, 2026 00:48
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.

shoot2: Explosions appear at wrong position when enemies are destroyed

3 participants