#2826 closed defect (fixed)
MI2: Inventory object cloning
| Reported by: | SF/dvitas | Owned by: | eriktorbjorn |
|---|---|---|---|
| Priority: | normal | Component: | Engine: SCUMM |
| Version: | Keywords: | ||
| Cc: | Game: | Monkey Island 2 |
Description
1) ScummVM version (scummvm -v): ScummVM 0.9.0 (Jun 22 2006 09:26:48) Features compiled in: Vorbis FLAC MP3 zLib MPEG2
2) Bug details, including instructions on reproducing: When on Dinkey Island try take the action "Pull" on object "rope" in inventory. This rope will clone;)
3) Language of game : English
4) Version of game : I'm not sure... ScummVM GUI shows it as (English (US)/DOS)
5) Platform and Compiler: platform = WinXP. Compiler = I'm not sure... I have downloaded WinXP binaries from SourceForge.
6) save game attached. Just "pull" the "rope" in inventory.
Ticket imported from: #1555938. Ticket imported from: bugs/2826.
Attachments (2)
Change History (15)
by , 18 years ago
| Attachment: | monkey2.s20 added |
|---|
comment:1 by , 18 years ago
That's a strange bug. And as far as I can tell, it does not happen with the original interpreter.
I think it's at least partly related to that you can pull the rope off the chest to pick it up. Sometimes when I tried it in a different room, ScummVM would also draw the chest - sans rope - as the rope was cloned. Other times, ScummVM would just crash with a "findObjectInRoom: Object 1047 not found in room ..." error.
So it seems like the rope's script to remove itself from the chest is run again (and again, and again), but I don't understand the handling of verbs well enough to figure out what's really happening here.
By the way, I can reproduce the error using my own old savegames, or boot params, so it's probably not just some once-in-a-blue-moon glitch.
comment:2 by , 18 years ago
I don't see any special case to stop the player from pulling the rope a second time, though I could very well be missing something.
If so, then perhaps there should be safeguards in o5_drawObject() and o5_pickupObject(), but does that really match the original behaviour? I thought MI2 was among the first - perhaps *the* first - SCUMM game to be supported by ScummVM, so it's hard to imagine there being a bug like that still there.
Interestingly, there's an o5_pickupObjectOld() which does guard against picking the same object up twice... but I couldn't find any trace of it in ScummVM 0.0.1, so I guess it's for different games. Maybe.
comment:3 by , 18 years ago
| Summary: | Inventory object cloning in MI2 → MI2: Inventory object cloning |
|---|
comment:4 by , 18 years ago
o5_pickupObjectOld is for old games, aye.
I wonder if this is an old bug or a regression. Would be interesting to try older ScummVM versions to see if they also suffer from this bug. Is there a good boot param one can use to reproduce this?
comment:5 by , 18 years ago
Boot param 997 is pretty close. I've only tried SVN, 0.9.0 and 0.8.0, and it happens with all of those, at least.
At one time, I used to have all stable releases of ScummVM on my computer, all the way back to 0.0.1. But then the soname of one of the libraries I linked against changed, and I upgraded to GCC 4.x which makes it difficult to compile at least some of the early versions. (The change to o5_jumpRelative() is easy to backport, of course, but I seem to recall there were other difficulties as well.)
comment:6 by , 18 years ago
Oh, and to try it in DOSbox:
* Type "monkeyspit" * Press Ctrl-D * Press Ctrl-L * Enter "997"
Actually, the bug seems to be *partly* there in the original as well. In most rooms, nothing happened when I pulled the rope. But in one case, the game did draw the image of the rope-less chest, even though the chest wasn't really in the room. That could indicate that ScummVM is running the correct script. Good. That simplifies things.
But unlike ScummVM, the original never crashed complaining that it couldn't find the object in the room. Nor did it "clone" the rope.
Kirben assured me the other day that our o5_pickupObject() matches the original disassembly. But of course, that opcode is implemented in terms of a number of other functions, and I don't think he extended the comparision to cover those as well.
comment:7 by , 18 years ago
I guess we can tentatively assume that the right scripts are executed. I think it's roomobj-97-1047, which is pretty short. In fact, I'm guessing that the part that handles pulling the rope is simply:
[001E] (05) drawObject(1045, setImage(2)); [0024] (5D) setClass(1045,[6]) [002B] (25) pickupObject(1047,0) [002F] (00) stopObjectCode()
I'm furthermore guessing that 1045 is the chest object, and that the first two lines are simply to redraw it and tell it that the rope is now gone from it.
Kirben assures me that the pickupObject() opcode is identical to the original, but does that also extend to the functions that o5_pickupObject() calls? Specifically, what about addObjectToInventory()? If I change it to a no-op when whereIsObject() returns WIO_INVENTORY, both the cloning and the "object not in room" crashes seem to go away.
The chest glitch remains, but that's as in the original.
Though I don't see any corresponding check in ScummVM 0.0.1...
comment:8 by , 18 years ago
This is definitely a script bug. In fact, I can't reproduce it, since all version of MI2 I have fix it. In my german PC version, the code in roomobj-97-1047 for event 6 (pull) looks as follows:
[001E] (90) VAR_RESULT = getObjectOwner(VAR_ME) [0023] (48) if (VAR_RESULT == 15) { [002A] (05) drawObject(1045, setImage(2)); [0030] (5D) setClass(1045,[6]) [0037] (25) pickupObject(1047,0) [003B] (**) } [003B] (00) stopObjectCode()
And in the english mac version I own, they simply don't allow you to "pull" the rope at all...
So, we should implement a workaround. One way would be to simply disallow "pull" on this object. Another would be to provide a patched script. Or we could add checks to both drawObject and pickupObject for this particular case...
comment:9 by , 18 years ago
I'd try to fix this myself, but since I own no version of MI2 exhibiting the problem, I have a hard time doing that.
Some more thoughts: To disallow "pull" on the rope, one could add a mod to getVerbEntrypoint() to make it fail to find the entrypoint 6 for object 1047, if the gameid is MI2... But only do that if the object is already in the inventory (mimicking the german PC version fix).
comment:10 by , 18 years ago
| Owner: | set to |
|---|---|
| Resolution: | → fixed |
| Status: | new → closed |
comment:11 by , 18 years ago
The verb entry workaround works for me. Committed, after discussing with Fingolfin.
save with "cloning rope" bug