Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCI32: Fix QFG4 inventory leaks across restarts #1440

Merged
merged 1 commit into from Dec 19, 2018

Conversation

Projects
None yet
2 participants
@Vhati
Copy link
Contributor

commented Dec 7, 2018

Fixes item properties that weren't getting reset, bug #10768

// Some inventory item properties leak across restarts. Most conspicuously, the
// torch icon appears pre-lit after a restart, if it had been lit before.
//
// script 16 - thePiepan (item #28): loop, cel, value
// script 35 - theBroom (item #39): cel, value
// script 35 - theTorch (item #44): cel
//
// Glory::restart() tries to revert a bunch of globals to their original state.
// Each value was individually loaded into acc and assigned (ldi+sag, ldi+sag,
// ldi+sag, etc).
//
// One range of globals could be distilled to multiples of 45. We optimize
// those with a loop. Another range was arbitrary. We stack up those values all
// at once, then loop over the stack to pop(), assign, and do the next global.
// We use the freed bytes to reset the 3 items' properties with a subroutine.

To test...

  • Create a character.
  • Get the affected items and the means to modify them.
# These first three are available in the starting room.
theThrowdagger: send hero get 5
theFlint:       send hero get 21
theTorch:       send hero get 44

theHair:        send hero 38
theBroom:       send hero 39

theBerries:     send hero get 30
thePiePan:      send hero get 28
  • Flint + torch = Torch icon will become lit.

  • Hair + broom = Broom's icon will have hair woven through it. Weight increases.

  • Berries + pan = Pan's icon will appear filled with ingredients. Weight increases.

  • Restart. New character. Get the items again.

  • Without this patch, the items' previous icon and weight will persist.

@digitall digitall requested review from bluegr and m-kiewitz Dec 8, 2018

@Vhati Vhati force-pushed the Vhati:qfg4_restart branch from c774893 to a60a7d3 Dec 11, 2018

0x39, 0x08, // pushi 8d (global[384] = 8)
0x39, 0x07, // pushi 7d (global[385] = 7)
0x39, 0x0a, // pushi 10d (global[386] = 10)
0x3c, // dup (global[387] = 10)

This comment has been minimized.

Copy link
@bluegr

bluegr Dec 16, 2018

Member

Why do you need to initialize all these parameters here?

This comment has been minimized.

Copy link
@Vhati

Vhati Dec 16, 2018

Author Contributor

The original asm loaded each value into acc and set specific globals, which happened to all be adjacent.

ldi&sag, ldi&sag, ldi&sag, ldi&sag...

I made room by optimizing.

push push push push all values at once... loop-pop() the stack N times, each time setting the next global. The loop is shorter than all those individual assignment ops.

This comment has been minimized.

Copy link
@bluegr

bluegr Dec 17, 2018

Member

Right. Please add that description to the comment, as it provides a better understanding of your changes

This comment has been minimized.

Copy link
@Vhati

Vhati Dec 17, 2018

Author Contributor

Description has been revised.

@Vhati Vhati force-pushed the Vhati:qfg4_restart branch from 5f46498 to a6b3298 Dec 17, 2018

SCI32: Fix QFG4 inventory leaks across restarts
Fixes item properties that weren't getting reset, bug #10768

@Vhati Vhati force-pushed the Vhati:qfg4_restart branch from a6b3298 to e3d2834 Dec 17, 2018

@bluegr

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

Thanks for your work! Merging

@bluegr bluegr merged commit a520c6e into scummvm:master Dec 19, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Vhati Vhati deleted the Vhati:qfg4_restart branch Dec 19, 2018

rsn8887 added a commit to rsn8887/scummvm that referenced this pull request Mar 20, 2019

SCI32: Fix QFG4 inventory leaks across restarts (scummvm#1440)
Fixes item properties that weren't getting reset, bug #10768

rsn8887 added a commit to rsn8887/scummvm that referenced this pull request May 2, 2019

SCI32: Fix QFG4 inventory leaks across restarts (scummvm#1440)
Fixes item properties that weren't getting reset, bug #10768
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.