I was rereading some of the older bug reports, particularly one that I
posted for RPG a while back. And it occurs to me that the unintuitive way of babies being generated and then killed during bomb explosions and other events also has effects in TCB.
First, a description of the problem. Tar is only ever removed one square at a time, and baby generation from unstable tar is dealt with at this time. There are no exceptions to this: stuff like simultaneous stabs only work because the game calculates what tar squares will be removed before it starts, and won't create babies under swords.
As a group of tar is destroyed, this leaves moments where there are pieces of tar that are going to be destroyed but are currently unstable. Currently, this tar is automatically converted into new babies. Then later, they can end up being destroyed by the same thing that caused the tar to become unstable in the first place.
As for what the current system affects... well, for starters, babies instantaneously generated and then killed by an explosion get added to the kill counter. More importantly, it fires the "
Tar/Mud/Gel baby formed"
events, which could cause scripts to run under conditions that weren't originally intended. Furthermore, it can cause "
Monster stabbed"
events if briar is destroying the newly formed babies.
In short, the order in which the tar is destroyed during something that should be considered 'instantaneous' (bomb explosions, briar, bridges over water) can cause differences in scripts.
EDIT: It can also affect whether some babies are generated at all: if a sword-wielding character is blocking where a baby will be generated, then the baby will be generated *only* if the character is destroyed by the explosion before it creates the unstable tar.
O
TT
TT
DTTD
\/
In the above example, two Decoys (D) have their swords on the final row of 2x4 piece of tar. The bomb (O) will explode destroying the decoys and all but the last row of tar. But only one baby will be created from the SW corner of tar.
===
Since this now concerns both TCB and RPG, I've been trying to think of a way to solve this. But if we try to remove *all* the tar before we generate babies, we immediately hit a big problem: backcompatibility.
TTTT TTTT
TTTT T***
TTTT T***
TTTT T***
O
12TT 12TT 12TT 1267
3 ** 3 ** 3 ** 3 8
T*** 4 ** 4 ** 4 **
T*** 56** 5 ** 5 **
123T 123T 123T 1237
4 ** 4 ** 4 ** 4 *
5*** 5 ** 5 ** 5 **
T*** 6*** 6 ** 6 **
Above is a diagram of a 4x4 piece of tar being having a 3x3 portion of itself blown up. Two rows then appear. The first row shows how baby generation currently occurs, and the second row shows how it might occur if we didn't test unstable tar until all of it is destroyed.
In the first row, each part of the explosion is dealt with going down each column, starting from the leftmost. Tar that is immediately unstable automatically generates babies, so those go first in the movement order. So when the NW corner of the explosion removes the first piece of tar, we immediately get to generate 3 babies. On the next tile, we generate three more babies (#4, #5 and #6). On the final tile in that column, we destroy baby #6.
If we destroyed all tar before processing unstable tar though, the current system is to test the 8 squares around each removed square of tar. So we'd remove all the tar, and then test the NW corner of the removed tar first, which immediately creates 5 babies. Already, a change in movement order. In short, the unexploded tar in the current code prevents certain babies from being created earlier than they might otherwise have been.
===
So, to both fix this problem *and* preserve the current movement order is somewhat problematic. The best way I can currently think of is to replace the AddNewMonster and CueEvent addition in RemoveStabbedTar with adding both the coordinates and tartype to a CoordStack/CoordIndex combination, which can then be iterated through to create the correct babies in the correct order. Furthermore, DestroyTar should be allowed to remove coordinates form this set. So you might have the following changes:
CDbRoom::RemoveStabbedTar
Click here to view the secret text
×
if (!IsValidColRow(i,j) ||
//Skip over the square that got stabbed or...
(i == wX && j == wY) ||
//...a square that doesn't contain tar or...
(GetTSquare(i, j) != wTarType) ||
//...a tar tile already marked as a future baby.
babyOrder.has(i,j))
continue;
//Get the orthogonal squares once for speed.
const bool bIsNorthTar = (j > 0) ? (GetTSquare(i, j - 1)==wTarType) && !babyOrder.has(i,j-1) : false;
const bool bIsSouthTar = (j < this->wRoomRows - 1) ? (GetTSquare(i, j + 1)==wTarType) && !babyOrder.has(i,j+1) : false;
const bool bIsWestTar = (i > 0) ? (GetTSquare(i - 1, j)==wTarType) && !babyOrder.has(i-1,j) : false;
const bool bIsEastTar = (i < this->wRoomCols - 1) ? (GetTSquare(i + 1, j)==wTarType) && !babyOrder.has(i+1,j) : false;
//Check the four corners.
if (
//Check northwest corner.
(i > 0 && j > 0 &&
GetTSquare(i - 1, j - 1)==wTarType &&
!babyOrder.has(i-1,j-1) &&
bIsNorthTar && bIsWestTar) ||
//Check northeast corner.
(i < this->wRoomCols - 1 && j > 0 &&
GetTSquare(i + 1, j - 1)==wTarType &&
!babyOrder.has(i+1,j-1) &&
bIsNorthTar && bIsEastTar) ||
//Check southwest corner.
(i > 0 && j < this->wRoomRows - 1 &&
GetTSquare(i - 1, j + 1)==wTarType &&
!babyOrder.has(i-1,j+1) &&
bIsSouthTar && bIsWestTar) ||
//Check southeast corner.
(i < this->wRoomCols - 1 && j < this->wRoomRows - 1 &&
GetTSquare(i + 1, j + 1)==wTarType &&
!babyOrder.has(i+1,j+1) &&
bIsSouthTar && bIsEastTar)
)
//Stable tar--skip to next square.
continue;
...
//Tar disappears.
DestroyTar(i, j, CueEvents);
//Mark the unstable tar for conversion.
if (!babyOrder.has(i,j))
{
// babyType.Add(i,j,wTarType);
babyOrder.Push(i,j);
}
if (!pMonster)
{
//Spawn a tarbaby.
const UINT wOSquare = GetOSquare(i,j);
//Don't create babies on swords, stairs, pits or the player.
if (!(swordCoords.Exists(i, j) || bIsStairs(wOSquare) || bIsPit(wOSquare) ||
(i == wSX && j == wSY)))
{
//Spawn a tarbaby.
CMonster *m = NULL;
switch (wTarType)
{
case T_TAR:
m = AddNewMonster(M_TARBABY,i,j);
CueEvents.Add(CID_TarBabyFormed, m);
break;
case T_MUD:
m = AddNewMonster(M_MUDBABY,i,j);
CueEvents.Add(CID_MudBabyFormed, m);
break;
case T_GEL:
m = AddNewMonster(M_GELBABY,i,j);
CueEvents.Add(CID_GelBabyFormed, m);
break;
default: ASSERT(!"Bad tar type"); continue;
}
if (this->pCurrentGame)
m->SetCurrentGame(this->pCurrentGame);
m->bIsFirstTurn = bDelayBabyMoves;
m->SetOrientation(sgn(wSX - i), sgn(wSY - j));
}
}
//This tar breaking may invalidate a previous decision,
//so recompute them.
goto recompute;
EDIT: Upon further reflection, the changes to RemoveStabbedTar would need to be more extensive, since if we removed unstable tar when creating 'babies in potentia', then additional explosions/briar growth would not detect that there is 'tar' to be destroyed on those squares, so the second routine of DestroyTar would not run. Instead, the new baby generation routine will have to use babyOrder to turn existing unstable tar into babies. This also means that we don't need babyType to store what type of baby was generated, but since we're not removing tar at this juncture, RemoveStabbedTar needs to treat 'babies in potentia' as tiles that *don't* have Tar. Changing GetTSquare to reflect this is out of the question, so I've updated the example code to try and reflect what might have to be done.
EDIT 2: The attached patch file is more accurate and has actually been compiled and undergone preliminary testing.
CDbRoom::DestroyTar
Click here to view the secret text
× ASSERT(bIsTar(GetTSquare(wX,wY)));
Plot(wX, wY, T_EMPTY);
if (babyOrder.has(wX,wY))
{
// babyType.Remove(wX,wY);
babyOrder.Remove(wX,wY);
}
ASSERT(this->wTarLeft); //This function should never be called with 0 Tar squares
babyType and babyOrder would need to be part of the CDbRoom class, and I'm not entirely sure what all needs to be updated to make that happen (not as simple as just adding them to DbRooms.h, since there's a lot of initialization and variable copying done). There would also need to be a new procedure to iterate over babyOrder and create the new babies, and this would need to be called by everything that uses RemoveStabbedTar. Things like bomb explosions and briar should delay calling this new procedure until after they're done though, rather than immediately after each RemoveStabbedTar call.
One change that should probably be done though is to forbid babies from being created on water. The current test used in RemoveStabbedTar is:
if (!(swordCoords.Exists(i, j) || bIsStairs(wOSquare) || bIsPit(wOSquare) || (i == wSX && j == wSY))). This means that babies can be created over water, but will immediately drown, and it also means that water can cause "
Tar baby formed"
events to fire even when no Tar babies live to the end of the turn, while pits can't. If we change it to prevent babies from being created on water, then that neatly fixes the problems for bridges, and so the CheckForFallingAt routine can safely attempt to create babies immediately after it destroys each piece of tarstuff.
===
In summary, I'm not entirely sure of the best way of solving this problem for both TCB and RPG, so these are my observations and understanding of it all. Hopefully it'll help.
EDIT 2: I've since coded a quick patch based on Build 84 source code, and have attached it to this post. I decided that the best variable to emulate with regards to initialization and copying was newFuses and the like, so that means one initialization in CDbRoom::Clear and one copy in CDbRoom::SetMembers. It looks to preserve movement order, and isn't causing Museum of Ooze to freak out. Haven't comprehensively tested it though. Also couldn't quickly think of a better name than babyOrder, so feel free to change that if you want.
[Last edited by TFMurphy at 11-09-2010 09:15 PM]