Kawa-oneechan / SCICompanion

SCI Companion - an editor for Sierra SCI games, altered.
18 stars 7 forks source link

Fix duplicate property assignment / atop p-code. #14

Closed laurence-myers closed 1 year ago

laurence-myers commented 1 year ago

I noticed that re-compiling a script would change (= state newState) into (= state (= state newState)). Looking at the p-code, I saw that the aTop instruction would be duplicated.

I found it was just caused by StoreProperty() being called twice when ENABLE_LDMSTM is set. I've fixed it by moving the original call into an #else branch.

Original

2023-04-03 19_34_44-SCICompanion -  MyButton_54 sc

2023-04-03 19_35_04-_new 1 - Notepad++

Duplicated

Compiling then de-compiling gives these results.

2023-04-03 19_37_10-SCICompanion -  MyButton_54 sc

2023-04-03 19_35_08-C__Users_L Dawg_AppData_Local_Temp_script sca txt - Notepad++

Kawa-oneechan commented 1 year ago

I have confirmed with a random script's state machine from my own The Dating Pool project and my experiments lab that includes the following test that your fix is valid and good.

(local
    fibs = [ 1 1 2 3 5 8 13 21 ]
)

(procedure (DerefTest &tmp ptr)
    (DbugStr "DerefTest: gonna try and mangle fibs array.")
    (= ptr @fibs) ; Grab a pointer to fibs

    ; Dereference ptr (that is, [fibs 0]) -- LDM
    (DbugStr "(= ptr @fibs) --> *ptr is %d" *ptr)

    ; Set [fibs 0] to 6 -- STM
    (= *ptr 6)
    (DbugStr "(= *ptr 6) --> *ptr is %d" *ptr)

    ; Point to [fibs 2] and set that to 7.
    (= ptr (+ ptr 2))
    (= *ptr 7)
    (DbugStr "(= ptr (+ ptr 2)) (= *ptr 7) --> *ptr is %d" *ptr)

    ; Expected fibs array: [ 6 1 7 2 3... ]
)

Original output was

DerefTest: gonna try and mangle fibs array.
(= ptr @fibs) --> *ptr is 1
(= *ptr 6) --> *ptr is 6
(= ptr (+ ptr 2)) (= *ptr 7) --> *ptr is 7

And the new output matches.