Skip to content

cache pointer to teleport mobj in sector#2684

Open
fabiangreffrath wants to merge 9 commits into
masterfrom
sector_teleport
Open

cache pointer to teleport mobj in sector#2684
fabiangreffrath wants to merge 9 commits into
masterfrom
sector_teleport

Conversation

@fabiangreffrath
Copy link
Copy Markdown
Owner

This is an alternative approach to the one presented in kraflab/dsda-doom#889. It makes use of the facts that (1) only the first MT_TELEPORTMAN type mobj in a sector is used for teleport respawning and (2) that mobjs are added to sectors in order. This means, the first time a MT_TELEPORTMAN type mobj is placed into a sector, this one will be used, all other MT_TELEPORTMAN type mobjs will be ignored.

This even takes care of the absurd situation that a MT_TELEPORTMAN type mobj is removed from a sector and of the even more absurd situation that another such mobj remains in the sector after that and thus becomes the active one. (You know, DEHACKED and stuff. Imagine a teleporter spitting Pain Elemental or whatever).

This approach survives our own demo test suite and the d411-780.lmp demo that revealed a typo in the implementation suggested for DSDA-Doom.

Regarding the performance impact. Run woof -file ~/Downloads/oversat.zip and load the attached savegame. Shoot the switch and turn around.

In my case, FPS went down from ~700 before hitting the switch down to ~8 afterwards on the master branch, and from ~700 down to ~200 on this branch.

Question remains, have I covered all cases which require to handle the new sector_t field? New Game, Load Game works. How about UDMF? Do I have to do anything for keyframes?

woofsav2.zip

Comment thread src/p_maputl.c Outdated
@elf-alchemist
Copy link
Copy Markdown
Collaborator

The existence of UDMF itself shouldn't cause any problems here. The real possible problem could come when handling parameterized/hexen/zdoom line specials, in the future for Woof! or in the present for DSDA, as they could work a bit differently.

As it happens, we were discussing this on the DCPC discord, and AFAICT this isn't an issue with those line specials, as it seems that non-zero Thing ID (TID) hashmaps are used to target teleport destinations instead of lowest index of the first tagged sector. So there's no major concern for compat in that case.

In the future, once the Bulk Demo Tester tool is more matured, we'll probaly be able to just use it for these kinds of things, testing changes against the existing DSDArchive.

@rfomin
Copy link
Copy Markdown
Collaborator

rfomin commented May 8, 2026

Do I have to do anything for keyframes?

Yes, we need to add saving of teleport field in both kf_memory.c and kf_file.c in ArchiveWorld/UnArchiveWorld functions.

@fabiangreffrath
Copy link
Copy Markdown
Owner Author

Yes, we need to add saving of teleport field in both kf_memory.c and kf_file.c in ArchiveWorld/UnArchiveWorld functions.

Do we have a means to store variable arrays in keyframes? 😬

@fabiangreffrath fabiangreffrath changed the title save pointer to first teleport mobj in sector save pointers to teleport mobjs in sector May 9, 2026
@fabiangreffrath
Copy link
Copy Markdown
Owner Author

Do we have a means to store variable arrays in keyframes? 😬

Wait, the whole array is just a single memory block allocated with PU_LEVEL, so it's sufficient to just save and restore that pointer, right?

@rfomin
Copy link
Copy Markdown
Collaborator

rfomin commented May 10, 2026

Do we have a means to store variable arrays in keyframes? 😬

Wait, the whole array is just a single memory block allocated with PU_LEVEL, so it's sufficient to just save and restore that pointer, right?

No, there will be a memory leak because we use realloc. We would need to save all elements in a keyframe, then delete and restore the array each time.

Is this optimization worth it, though? It seems like a specific case to me.

@fabiangreffrath
Copy link
Copy Markdown
Owner Author

Is this optimization worth it, though? It seems like a specific case to me.

WDYM, the optimization in general or this implementation with the dynamic arrays?

@fabiangreffrath
Copy link
Copy Markdown
Owner Author

I have updated the Oversat.wad savegame for the current state of the implementation:

woofsav3.zip

@fabiangreffrath
Copy link
Copy Markdown
Owner Author

I have updated the Oversat.wad savegame for the current state of the implementation:

Yay for savegames in compressed JSON format, btw. 😉

@fabiangreffrath fabiangreffrath changed the title save pointers to teleport mobjs in sector buffer pointer to teleport mobjs in sector May 11, 2026
@fabiangreffrath fabiangreffrath changed the title buffer pointer to teleport mobjs in sector buffer pointer to teleport mobj in sector May 11, 2026
@fabiangreffrath fabiangreffrath changed the title buffer pointer to teleport mobj in sector cache pointer to teleport mobj in sector May 12, 2026
@fabiangreffrath
Copy link
Copy Markdown
Owner Author

Is this optimization worth it, though? It seems like a specific case to me.

It was a highly inefficient and redundant linear search through all thinkers. I have replaced it with a per-sector array which caches each sector's first teleport spawn mobj. Please have a look and tell me what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants