[Scummvm-git-logs] scummvm branch-2-3 -> b476d645f9e968aab3d587ea749e15d73f0b6a2a
mduggan
mgithub at guarana.org
Wed Sep 22 22:47:57 UTC 2021
This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .
Summary:
8fcfd9cd6f ULTIMA8: Use 2 bytes of intrinsic stack for UINT8
b476d645f9 ULTIMA8: Put back ARG_NULL8 macro with fix
Commit: 8fcfd9cd6f139cfe0aaf2f3ae50a7a14cfad15cc
https://github.com/scummvm/scummvm/commit/8fcfd9cd6f139cfe0aaf2f3ae50a7a14cfad15cc
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2021-09-23T07:47:35+09:00
Commit Message:
ULTIMA8: Use 2 bytes of intrinsic stack for UINT8
This fixes #12917.
The usecode "push byte" opcodes (0x0A and 0x5D) always push 16 bits on the
stack, but the ARG_UINT8 macro was only removing 1 byte. Most of the time this
went unnoticed because the UINT8 was the last argument in most cases (eg, a z
val), or unused.
This led to an inconsistency where sometimes z values were being popped with
ARG_UINT16 and sometimes ARG_UINT8. The original games do not support z values
beyond 254, so this should always be UINT8.
Additionaly, a while back "push byte" was fixed so it always sign extends, but
this could result in a case where we pop incorrect values. For example, a high
Z value could get sign-extended on push, and then popped back as a UINT16,
giving a z of 64000ish.
Fix by always moving the SP by 2 bytes, only use the first one.
Correcting this also fixes the strange color ordering I thought was needed for
I_jumpToAllGivenColor in Crusader: No Regret, where actually it should have
been popping the values as 16-bit instead of 8-bit.
Changed paths:
engines/ultima/ultima8/graphics/palette_fader_process.cpp
engines/ultima/ultima8/usecode/intrinsics.h
engines/ultima/ultima8/world/actors/actor.cpp
engines/ultima/ultima8/world/current_map.cpp
engines/ultima/ultima8/world/fireball_process.cpp
engines/ultima/ultima8/world/item.cpp
diff --git a/engines/ultima/ultima8/graphics/palette_fader_process.cpp b/engines/ultima/ultima8/graphics/palette_fader_process.cpp
index cb0aab5f37..4a46a6632f 100644
--- a/engines/ultima/ultima8/graphics/palette_fader_process.cpp
+++ b/engines/ultima/ultima8/graphics/palette_fader_process.cpp
@@ -253,9 +253,9 @@ uint32 PaletteFaderProcess::I_fadeToGivenColor(const uint8 *args,
else if (_fader) _fader->terminate();
// TODO: guessing that color order should be same as other one below?
- ARG_UINT8(b);
ARG_UINT8(r);
ARG_UINT8(g);
+ ARG_UINT8(b);
ARG_UINT16(nsteps);
ARG_UINT16(unk);
@@ -326,9 +326,9 @@ uint32 PaletteFaderProcess::I_jumpToAllGivenColor(const uint8 *args,
if (_fader && _fader->_priority > 0x7FFF) return 0;
else if (_fader) _fader->terminate();
- ARG_UINT8(b);
ARG_UINT8(r);
ARG_UINT8(g);
+ ARG_UINT8(b);
// Transform matrix goes 0~2048, scale 0-63 vals from input
const int16 r16 = static_cast<int16>(r) * 32;
diff --git a/engines/ultima/ultima8/usecode/intrinsics.h b/engines/ultima/ultima8/usecode/intrinsics.h
index 049ea78814..644fdbad36 100644
--- a/engines/ultima/ultima8/usecode/intrinsics.h
+++ b/engines/ultima/ultima8/usecode/intrinsics.h
@@ -34,7 +34,9 @@ typedef uint32(*Intrinsic)(const uint8 *args, unsigned int argsize);
// TODO: range checking on args
-#define ARG_UINT8(x) uint8 x = (*args++);
+// UINT8s are pushed on the stack as words (see push byte opcodes),
+// so we ignore the top byte when popping.
+#define ARG_UINT8(x) uint8 x = (*args++); args++;
#define ARG_UINT16(x) uint16 x = (*args++); x += ((*args++) << 8);
#define ARG_UINT32(x) uint32 x = (*args++); x += ((*args++) << 8); \
x+= ((*args++) << 16); x += ((*args++) << 24);
@@ -88,7 +90,6 @@ typedef uint32(*Intrinsic)(const uint8 *args, unsigned int argsize);
WorldPoint x; \
UCMachine::get_instance()->dereferencePointer(ucptr_##x, x._buf, 5);
-#define ARG_NULL8() args+=1;
#define ARG_NULL16() args+=2;
#define ARG_NULL32() args+=4;
diff --git a/engines/ultima/ultima8/world/actors/actor.cpp b/engines/ultima/ultima8/world/actors/actor.cpp
index e406bc37f6..42178976bc 100644
--- a/engines/ultima/ultima8/world/actors/actor.cpp
+++ b/engines/ultima/ultima8/world/actors/actor.cpp
@@ -2545,8 +2545,8 @@ uint32 Actor::I_pathfindToPoint(const uint8 *args, unsigned int /*argsize*/) {
ARG_ACTOR_FROM_PTR(actor);
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_UINT16(z);
- ARG_NULL16(); // unknown. Only one instance of this in U8, value is 5.
+ ARG_UINT8(z);
+ ARG_NULL16(); // unknown. Only one instance of this in U8, values are 5,1.
if (!actor) return 0;
if (GAME_IS_CRUSADER) {
diff --git a/engines/ultima/ultima8/world/current_map.cpp b/engines/ultima/ultima8/world/current_map.cpp
index d4a4b7452b..e84efcf777 100644
--- a/engines/ultima/ultima8/world/current_map.cpp
+++ b/engines/ultima/ultima8/world/current_map.cpp
@@ -1352,7 +1352,7 @@ uint32 CurrentMap::I_canExistAt(const uint8 *args, unsigned int argsize) {
ARG_UINT16(shape);
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_UINT16(z);
+ ARG_UINT8(z);
if (argsize > 8) {
//!! TODO: figure these out
ARG_UINT16(unk1); // is either 1 or 4
diff --git a/engines/ultima/ultima8/world/fireball_process.cpp b/engines/ultima/ultima8/world/fireball_process.cpp
index 93ffe69683..fa6e1ad72e 100644
--- a/engines/ultima/ultima8/world/fireball_process.cpp
+++ b/engines/ultima/ultima8/world/fireball_process.cpp
@@ -173,7 +173,7 @@ uint32 FireballProcess::I_TonysBalls(const uint8 *args,
ARG_NULL16(); // unknown
ARG_SINT16(x);
ARG_SINT16(y);
- ARG_UINT16(z);
+ ARG_UINT8(z);
Item *ball = ItemFactory::createItem(260, 4, 0, Item::FLG_FAST_ONLY,
0, 0, 0, true);
diff --git a/engines/ultima/ultima8/world/item.cpp b/engines/ultima/ultima8/world/item.cpp
index fa732abec5..b0e7cffa7e 100644
--- a/engines/ultima/ultima8/world/item.cpp
+++ b/engines/ultima/ultima8/world/item.cpp
@@ -3160,7 +3160,7 @@ uint32 Item::I_legalCreateAtCoords(const uint8 *args, unsigned int /*argsize*/)
ARG_UINT16(frame);
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_UINT16(z);
+ ARG_UINT8(z);
if (GAME_IS_CRUSADER) {
x *= 2;
@@ -3357,7 +3357,7 @@ uint32 Item::I_push(const uint8 *args, unsigned int /*argsize*/) {
return 0;
#if 0
- perr << "Pushing item to ethereal void: " << item->getShape() << "," << item->getFrame() << Std::endl;
+ perr << "Pushing item to ethereal void: id: " << item->getObjId() << " shp: " << item->getShape() << "," << item->getFrame() << Std::endl;
#endif
item->moveToEtherealVoid();
@@ -3423,7 +3423,7 @@ uint32 Item::I_popToCoords(const uint8 *args, unsigned int /*argsize*/) {
ARG_NULL32(); // ARG_ITEM_FROM_PTR(item); // unused
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_UINT16(z);
+ ARG_UINT8(z);
World *w = World::get_instance();
@@ -3521,7 +3521,7 @@ uint32 Item::I_move(const uint8 *args, unsigned int /*argsize*/) {
ARG_ITEM_FROM_PTR(item);
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_UINT16(z);
+ ARG_UINT8(z);
if (!item)
return 0;
@@ -3881,7 +3881,7 @@ uint32 Item::I_explode(const uint8 *args, unsigned int argsize) {
uint32 Item::I_igniteChaos(const uint8 *args, unsigned int /*argsize*/) {
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_NULL8();
+ ARG_UINT8(z); // unused
assert(GAME_IS_U8);
Commit: b476d645f9e968aab3d587ea749e15d73f0b6a2a
https://github.com/scummvm/scummvm/commit/b476d645f9e968aab3d587ea749e15d73f0b6a2a
Author: Matthew Duggan (mgithub at guarana.org)
Date: 2021-09-23T07:47:44+09:00
Commit Message:
ULTIMA8: Put back ARG_NULL8 macro with fix
Removed this macro in dc17170 because it had one usage and was wrong anyway,
but it's nice to avoid the unused variable warning.
Changed paths:
engines/ultima/ultima8/usecode/intrinsics.h
engines/ultima/ultima8/world/item.cpp
diff --git a/engines/ultima/ultima8/usecode/intrinsics.h b/engines/ultima/ultima8/usecode/intrinsics.h
index 644fdbad36..3049d8a0f8 100644
--- a/engines/ultima/ultima8/usecode/intrinsics.h
+++ b/engines/ultima/ultima8/usecode/intrinsics.h
@@ -90,6 +90,8 @@ typedef uint32(*Intrinsic)(const uint8 *args, unsigned int argsize);
WorldPoint x; \
UCMachine::get_instance()->dereferencePointer(ucptr_##x, x._buf, 5);
+// See comment on ARG_UINT8 for why +2 on NULL8
+#define ARG_NULL8() args+=2;
#define ARG_NULL16() args+=2;
#define ARG_NULL32() args+=4;
diff --git a/engines/ultima/ultima8/world/item.cpp b/engines/ultima/ultima8/world/item.cpp
index b0e7cffa7e..ffdb13554e 100644
--- a/engines/ultima/ultima8/world/item.cpp
+++ b/engines/ultima/ultima8/world/item.cpp
@@ -3881,7 +3881,7 @@ uint32 Item::I_explode(const uint8 *args, unsigned int argsize) {
uint32 Item::I_igniteChaos(const uint8 *args, unsigned int /*argsize*/) {
ARG_UINT16(x);
ARG_UINT16(y);
- ARG_UINT8(z); // unused
+ ARG_NULL8(); // z, unused
assert(GAME_IS_U8);
More information about the Scummvm-git-logs
mailing list