Also include room palette fixes for games that were saved with a different video mode. Unfortunately the scripts make changes to the room palette based on VAR_VIDEOMODE. The original interpreter does not fix that.
In the end, once fully understood, the CGA rendering is very similar for all versions. So I did manage to clean out some code. Hercules mode for MI1EGA is broken here, I will have another commit for that, once I understand how it works.
(original bug)
In LOOM, the scripts actually read the videomode var and make actor palette adjustments based on that. Which means for games saved in EGA mode, with the wrong videomode var value, there will be glitches.
This concerns not only Bobbin, but the script does things for various actors and costumes in various situations.
We just set the videomode var to the actual config after loading.
For Bobbin in normal costume, I also include a post-load fix for savegames that will not have the actor palettes fixed by the scripts (savegames that we allow in situations where the original would prevent them).
In most (if not all) versions of Loom before the Talkie v4 release, one
of the shepherds should trigger a "We are the masters of stealth" line
if Bobbin tries to use the stealth draft on them, but no proper value for
the actor number is ever given. In the original interpreters, the invalid
line would be skipped. The Talkie release changes the lines a bit and
makes the third shepherd (act. 4) say something similar, instead.
Until now, ScummVM would work around this by forcing the leftmost (act. 2)
shepherd to say this line. But, looking at the original script, it seems
that the original intent may have been to let any of the four shepherds
say this line, since script 232 expects a parameter which could be given
by any of the associated 422--425 (act. 2--5) actor objects.
So, moving this check from actorTalk() to o5_startScript() lets us
implement a more comprehensive workaround (with a bit more safety checks,
since there are so many Loom versions), but it also exposes another bug
in some EGA versions and derivatives (e.g. the French EGA version has it,
but the English EGA 1.1 version doesn't): if Bobbin uses the stealth draft
on the second shepherd (act. 3), then some line(s) will be completely
missing, potentially because this actor has been removed from the scene
in the earliest versions, although he's still in use.
Having zero reaction from the shepherds when you try their draft upon
them can be extremely confusing, so we need to work around this, too.
Forcing this actor to "stay" doesn't fix the issue for now (and the
symptoms are a bit different between ScummVM and the original
interpreters), so at the moment we force this workaround even if
`_enableEnhancements` is not enabled, when we detect this strange
actor behavior.
The direction parameter used in setDirection can be > 360, so it may not be passed to newDirToOldDir() directly.
Also, fix the converting routine for old saves.
(see https://bugs.scummvm.org/ticket/12499#comment:11)
In Actor::startWalkActor() we call adjustXYToBeInBox() twice, first with the dest coords, then again with the resulting coords from the first call. In the example from the bug ticket (clicking on Kenny the lemonade selling pirate) this will adjust the x position from 503 to 501 on the first pass and from 501 to 500 on the second pass.
The original SCUMM 5 and 6 (I checked MI2 and SAM) actually do it exactly like that, so it becomes kind of obvious where our code originates from.
However, for SCUMM 7 and 8 (I checked FT, DIG, COMI) the function has been simplified considerately. It makes the call to adjustXYToBeInBox() only once (and no call to checkXYInBoxBounds() either), so in our COMI example the x position will stay at 501.
Just something I came across in the disasm while doing something else. I also ran the original CJK interpreter in VirtualBox to confirm this weirdness...
(final fix for ticket no. 3215 ("SCUMM: Zak McKracken - intro behavior + movement")
The ticket has been closed a couple weeks ago, since a user claimed that the intro was fully fixed. And it mostly was. Except one little thing about the movement of the floating hat which, after chasing Zak, would return to the left of the screen one step to early.
Turns out that o2_waitForActor() got triggered one step to early, because we didn't set the final MF_TURN flag...
After my recent effort to do this for SCUMM3 I now try to achieve the same thing for v1/2. (Unsurprisingly) the step calculations actually have more in common with SCUMM3 than with the later versions upon which the code was based. However, I find the v1/2 code somewhat more difficult to fix than v3, since it is quite heavily twisted and refactored to fit into our common code. So all testing and bug reporting is welcome...
This is a follow-up commit to 8cbcde0c (bug
https://bugs.scummvm.org/ticket/12666). While that commit improved the situation, in particular the behavior described in the bug ticket, it still wasn't anything like pixel-perfect.
The agents would walk in a weird half circle due to being showered with walkActorToActor calls from the script.
I have now implemented the original walk code and also compared the first part of the INDY3 scene described in the ticket frame by frame with the DOSBox debugger to check whether it is truly pixel-perfect.
The particular scene with Indy and Donovan's men had all sorts of pathfinding issues compared to DOSBox:
- Indy's initial facing was downwards instead of to the right when walking from the window to the "meeting point" and the path he walked wasn't exactly correct.
- One of Donovan's men was facing down instead of left.
- Indy would take several weird up and down walks at the walk box transition from box 1 to box 3
- Indy would walk over the grass instead of on the road.
All fixes are from disasm. Some code I have removed (or rather commented out) seems to have been meant as a fix for bug no. 1778. So maybe that one has to be fixed again (correctly), but I don't know that yet.
The scene is not 100% fixed yet. Donovan's men walk a bit too much upwards in the beginning. But I can do that separately...
It will really be necessary to do some testing (and possibly more fixing) in the catacombs, in castle brunwald and in the zeppelin maze.
This is only supposed to apply to SCUMM3. It is a bit difficult to recognize this from SCUMM1/2 disasm, since things happen in a different order there. But it actually causes glitches in SCUMM1/2 (actors are slowed down sometimes) and it isn't required for bugs like no. 12278 (which has been fixed for SCUMM1/2 by one of my other recent commits).