monster awareness state not saved in savegames?

Hi all,

I have no idea if this is already known or not, but I just made an observation in Chocolate Doom that puzzles me: Apparently, the awareness states of the monsters in a map are not saved in savegames.

For example, in MAP01 cheat yourself the shotgun (IDFA), kill the first two zombies in the entrance hall at once and save a game immediately after they died. Of course, your firing of the shotgun will have woken up all the other zombies on the floor and they will approach to attack you.

Now load the saved game and wait for the zombies. They won't come for you, because they are still in their waiting state.

Is this expected?

Share this post


Link to post
Blue Shadow said:

Indeed, thank you. For some reasons, I was unable to find it in the list of known bugs myself. I already believed I was rather versed with regard to Vanilla Doom bugs now, but apparently I was wrong. :/

Share this post


Link to post

Are there any limit removing, or even raising, ports that don't fix this? (Crispy Doom does not. Neither does Doom+ obviously.)

Share this post


Link to post
plums said:

Are there any limit removing, or even raising, ports that don't fix this? (Crispy Doom does not. Neither does Doom+ obviously.)


I haven't been successful in applying a working solution for this to Doom Retro [yet].

Share this post


Link to post

If I understand the doomwiki article correctly, an easy half-measure might be to set non-null targets to target the player on game load, rather than setting all targets to null. This would at least make sure that active monsters stay active, which is the most important aspect IMHO - as purist said, resetting monsters to be asleep can break monster closets and other traps.

Share this post


Link to post
plums said:

If I understand the doomwiki article correctly, an easy half-measure might be to set non-null targets to target the player on game load, rather than setting all targets to null. This would at least make sure that active monsters stay active, which is the most important aspect IMHO - as purist said, resetting monsters to be asleep can break monster closets and other traps.

Just don't repeat the same mistake as Strife and do it before the player has been unarchived... needs to be done in a second sweep after all mobj_t's have been loaded, not during their loading.

Share this post


Link to post

Serialization and making sure to save ALL the important data sounds like a nightmare that you need to keep tabs on.

Share this post


Link to post

It possible to conditionally reconstruct infighting targets -you can find out what each prev/next pointer of each monster's mobj_t means if you take into account that monsters are serialized according to their linked list order. This way, you can find you which monster is which prev/next pointer (you can treat the raw pointer value as a sort of unique hashtag for each monster), and thus you can see what it was infighting with -if non null.

I'm doing this in Mocha Doom, and it's surpringly trivial to do. It does involve some boring search & replace code though, and having a hashtable/hashmap implementation at hand makes your life easier.

However, it's not without limitations: in order to be 100% certain that a certain mobj_t used to have a certain codepointer in memory, you must make certain that both the mobj_t before it and the mobj_t after it pointed to the same memory region.

//
// P_ArchiveThinkers
//
void P_ArchiveThinkers (void)
{
    thinker_t*		th;
    mobj_t*		mobj;
	
    // save off the current thinkers
    for (th = thinkercap.next ; th != &thinkercap ; th=th->next)
    {
	if (th->function.acp1 == (actionf_p1)P_MobjThinker)
	{
	    *save_p++ = tc_mobj;
	    PADSAVEP();
	    mobj = (mobj_t *)save_p;
	    memcpy (mobj, th, sizeof(*mobj));
	    save_p += sizeof(*mobj);
	    mobj->state = (state_t *)(mobj->state - states);
	    
	    if (mobj->player)
		mobj->player = (player_t *)((mobj->player-players) + 1);
	    continue;
	}
		
	// I_Error ("P_ArchiveThinkers: Unknown thinker function");
    }

    // add a terminating marker
    *save_p++ = tc_end;	
}
If all thinkers were continguous in memory:
Thinker(1) <-> Thinker(2) <-> Thinker(3) <-> ... <-> Thinker(n)
then the condition is trivially true.

But in the general case there will be mixed thinker and non-thinker objects:
Thinker(1) <-> Thinker(2) <-> Non-Thinker(1) <-> Thinker(3) <-> Non-Thinker (2) <-> Thinker(4) <-> Thinker(5) <-> Thinker(6)<-> etc.
So, in this case, it's NOT possible to unambiguously determine Thinker(1), Thinker(2), Thinker(3)'s and Thinker(4)'s memory pointers/hashes. But it IS possible to determine Thinker(5)'s codepointer, since both Thinker(4)->next and Thinker(5)->prev will be pointing at the same memory location.

So, in the general case, at best you can partially reconstruct codepointer.

HOWEVER, there's an element in our favour: since monster thinkers are spawned all together (in sequence) when the map starts, they are guaranteed to be contiguous in memory, at least as long as they are alive. But even dead monsters and decorations get a thinker....the only real problem is when there are respawning/resurrected or generated monsters on a map (archviles, PEs etc.) as well as a lot of temporary items like projectiles and active specials, those can complicate codepointer reconstruction A LOT, and if you're not careful you may get side-effects like monsters trying to attack projectiles or doors.

This would be made MUCH easier if each mobj_t included a pointer TO ITSELF, at least, but the only one doing so (indirectly) is the player's.

Share this post


Link to post
Maes said:

you may get side-effects like monsters trying to attack projectiles or doors.

Doors? Maybe (it seems unlikely, though). Projectiles? Impossible. I've actually seen monsters target projectiles (and dangling pointers left by exploded projectiles) via DeHackEd and they never attack them. I believe this is because A_Chase() checks for MF_SHOOTABLE before attacking, and monsters will therefore never attack projectiles (or any non-shootable map objects) they target.

Share this post


Link to post

They still may attempt to target them, even though the action will be nullified as invalid.

OK, so maybe attacking doors isn't so easy, but if pointer recreation is done without paying attention that the pointed entities are actually MobjThinker themselves, you may get monsters trying to attack thinkers that are not monsters or legitimate targets. That might get caught and cancelled by the engine, or might lead to a crash.

E.g. the archived thinker->prev and thinker->next pointers of some mobj_t may point at something unrelated, like decoration, a barrel, sector specials, etc. which are still thinkers, or even monsters that they had no gripe with. This might also be the only way that monsters can be made to attack e.g. the Icon of Sin, Archviles and PEs.

In any case, my point was that perfect pointer reconstruction is not always possible. The way Doom works, you're only guaranteed that the INITIAL thinkers on a map (that includes the player, monsters AND decorations) as well as their corpses will form a continuous linked list in memory. However, any monsters respawned, created etc. can cause the order to get pretty fucked up, so pointer reconstruction will only work in a relatively small set of cases.

Share this post


Link to post
Maes said:

They still may attempt to target them, even though the action will be nullified as invalid.

Find a known mobj_t and get the value it has in mo->thinker.function. All other mobj_t's will have the same value for it. Non-mobj thinkers will have other values. Simple.

There are not any non-thinker objects in the thinker list so I don't know why your double linked list illustration says "non-THINKER"; I believe you meant non-MOBJ.

Share this post


Link to post

I stand corrected. I should have said "thinkers that don't have the P_MobjThinker coeepointer". Those will cause breaks in the thinkers' linked list's continuity when serializing it.

Share this post


Link to post

Has anyone yet evaluated how this is handled in PrBoom+? Is there any way to reliably fix this without breaking Vanilla savegame format?

Share this post


Link to post
Maes said:

I stand corrected. I should have said "thinkers that don't have the P_MobjThinker coeepointer". Those will cause breaks in the thinkers' linked list's continuity when serializing it.

Even the non-Mobj ones will still eventually point at an Mobj. Let's consider if you have a Light flash thinker in the list...

thinkercap <-> mobj_t <-> lightflash_t <-> mobj_t <-> thinkercap
The address of the second mobj is in the lightflash_t's thinker.next, so if mobj #1 is targeting that address, you know it is targeting the second mobj. I'm not sure where if at all that you'd encounter a problem obtaining a complete mapping of mobj addresses when I think about it. It's a double linked list so every object is pointed at. You might run into problems in the following odd circumstances though:
* Fire flicker thinkers aren't saved in vanilla, and would create gaps in the list
* Objects that reference deleted thinkers are possible, and you won't find the deleted thinkers in the list at all.

EDIT: I have forgotten that thinkers other than mobjs might not be serialized wholescale in the way they are... Don't have time to go back and look at the code right now >_>

Share this post


Link to post
fabian said:

Has anyone yet evaluated how this is handled in PrBoom+? Is there any way to reliably fix this without breaking Vanilla savegame format?


Boom fixed this by indexing the thinkers when saving and restoring the pointers from the index when loading. Of course this breaks the savegame format, but for Boom that did not matter as it made several other changes to the map structures for its extended features that also needed saving.

Share this post


Link to post

I think I have found a very simple, universal and Vanilla compatible solution to this problem: I iterate through all available thinkers and assign each an increasing 32-bit integer "index" starting at 1, so I get a unique assignment between a thinker pointer and its index. When saving a mobj_t in saveg_write_mobj_t(), each pointer in the mobj->target and mobj->tracer fields gets replaced by its corresponding index. When loading the game again, after all thinkers have been restored, this procedere is reversed and each index in the mobj->target and mobj->tracer fields gets replaced again with the corresponding new thinker pointer.

This has worked perfectly fine for me so far, even on Frankenmaps like nuts.wad where everything infights everything. And the best is, it is complete Vanilla compatible, since the mobj->target and mobj->tracer fields may contain garbage from Vanilla's point of view, but they get discarded and overwritten with NULL anyway.

I will post my patch for further discussion here:

diff --git a/src/doom/g_game.c b/src/doom/g_game.c
index 608869c..a5ea2aa 100644
--- a/src/doom/g_game.c
+++ b/src/doom/g_game.c
@@ -1758,6 +1758,7 @@ void G_DoLoadGame (void)
     P_UnArchiveWorld (); 
     P_UnArchiveThinkers (); 
     P_UnArchiveSpecials (); 
+    P_RestoreTargets ();
  
     if (!P_ReadSaveGameEOF())
 	I_Error ("Bad savegame");
diff --git a/src/doom/p_saveg.c b/src/doom/p_saveg.c
index 45221c5..244250f 100644
--- a/src/doom/p_saveg.c
+++ b/src/doom/p_saveg.c
@@ -505,7 +505,7 @@ static void saveg_write_mobj_t(mobj_t *str)
     saveg_write32(str->movecount);
 
     // struct mobj_s* target;
-    saveg_writep(str->target);
+    saveg_writep((void *)(uintptr_t) P_ThinkerToIndex((thinker_t *) str->target));
 
     // int reactiontime;
     saveg_write32(str->reactiontime);
@@ -530,7 +530,7 @@ static void saveg_write_mobj_t(mobj_t *str)
     saveg_write_mapthing_t(&str->spawnpoint);
 
     // struct mobj_s* tracer;
-    saveg_writep(str->tracer);
+    saveg_writep((void *)(uintptr_t) P_ThinkerToIndex((thinker_t *) str->tracer));
 }
 
 
@@ -1653,8 +1653,6 @@ void P_UnArchiveThinkers (void)
 	    mobj = Z_Malloc (sizeof(*mobj), PU_LEVEL, NULL);
             saveg_read_mobj_t(mobj);
 
-	    mobj->target = NULL;
-            mobj->tracer = NULL;
 	    P_SetThingPosition (mobj);
 	    mobj->info = &mobjinfo[mobj->type];
 	    mobj->floorz = mobj->subsector->sector->floorheight;
@@ -1671,6 +1669,62 @@ void P_UnArchiveThinkers (void)
 
 }
 
+uint32_t P_ThinkerToIndex (thinker_t* thinker)
+{
+    thinker_t*	th;
+    uint32_t	i;
+
+    if (!thinker)
+	return 0;
+
+    for (th = thinkercap.next, i = 1; th != &thinkercap; th = th->next, i++)
+    {
+	if (th->function.acp1 == (actionf_p1) P_MobjThinker)
+	{
+	    if (th == thinker)
+		return i;
+	}
+    }
+
+    return 0;
+}
+
+thinker_t* P_IndexToThinker (uint32_t index)
+{
+    thinker_t*	th;
+    uint32_t	i;
+
+    if (!index)
+	return NULL;
+
+    for (th = thinkercap.next, i = 1; th != &thinkercap; th = th->next, i++)
+    {
+	if (th->function.acp1 == (actionf_p1) P_MobjThinker)
+	{
+	    if (i == index)
+		return th;
+	}
+    }
+
+    return NULL;
+}
+
+void P_RestoreTargets (void)
+{
+    mobj_t*	mo;
+    thinker_t*	th;
+    uint32_t	i;
+
+    for (th = thinkercap.next, i = 1; th != &thinkercap; th = th->next, i++)
+    {
+	if (th->function.acp1 == (actionf_p1) P_MobjThinker)
+	{
+	    mo = (mobj_t*) th;
+	    mo->target = (mobj_t*) P_IndexToThinker((uintptr_t) mo->target);
+	    mo->tracer = (mobj_t*) P_IndexToThinker((uintptr_t) mo->tracer);
+	}
+    }
+}
 
 //
 // P_ArchiveSpecials
diff --git a/src/doom/p_saveg.h b/src/doom/p_saveg.h
index 2d7beba..407f574 100644
--- a/src/doom/p_saveg.h
+++ b/src/doom/p_saveg.h
@@ -55,6 +55,10 @@ void P_UnArchiveThinkers (void);
 void P_ArchiveSpecials (void);
 void P_UnArchiveSpecials (void);
 
+uint32_t P_ThinkerToIndex (thinker_t* thinker);
+thinker_t* P_IndexToThinker (uint32_t index);
+void P_RestoreTargets (void);
+
 extern FILE *save_stream;
 extern boolean savegame_error;
 

Share this post


Link to post

Wow, very good. How does this work when loading a vanilla save game? Obviously the targets can't be restored, so I assume they are just set to NULL as in vanilla?

Share this post


Link to post
plums said:

Wow, very good. How does this work when loading a vanilla save game? Obviously the targets can't be restored, so I assume they are just set to NULL as in vanilla?

Exact. Since none of the pointers left in the mobj->targets fields by Vanilla will match an index in Crispy (well, let's say the propability is at least close to zero), there isn't anything returned from within the for-loop and NULL is returned as in Vanilla.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!


Register a new account

Sign in

Already have an account? Sign in here.


Sign In Now