From 59fb3ed900dd4e90b39b75cd7ba247180f94a662 Mon Sep 17 00:00:00 2001 From: Ashnal Date: Sat, 10 Sep 2022 13:29:18 -0400 Subject: [PATCH 01/11] Attempt to fix use after free bug with precipitation mobjs on netgame load --- src/p_mobj.c | 28 +++++++++++++++++++++------- src/p_saveg.c | 8 +++++++- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/src/p_mobj.c b/src/p_mobj.c index 530dd37d..befcf04f 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -10298,20 +10298,34 @@ void P_RemovePrecipMobj(precipmobj_t *mobj) void P_RemoveSavegameMobj(mobj_t *mobj) { // unlink from sector and block lists - P_UnsetThingPosition(mobj); - - // Remove touching_sectorlist from mobj. - if (sector_list) + if (((thinker_t *)mobj)->function.acp1 == (actionf_p1)P_NullPrecipThinker) { - P_DelSeclist(sector_list); - sector_list = NULL; + P_UnsetPrecipThingPosition((precipmobj_t *)mobj); + + if (precipsector_list) + { + P_DelPrecipSeclist(precipsector_list); + precipsector_list = NULL; + } + } + else + { + // unlink from sector and block lists + P_UnsetThingPosition(mobj); + + // Remove touching_sectorlist from mobj. + if (sector_list) + { + P_DelSeclist(sector_list); + sector_list = NULL; + } } // stop any playing sound S_StopSound(mobj); // free block - P_RemoveThinker((thinker_t *)mobj); + P_RemoveThinkerDelayed((thinker_t *)mobj); // Call directly here since we are calling P_InitThinkers R_RemoveMobjInterpolator(mobj); } diff --git a/src/p_saveg.c b/src/p_saveg.c index f79b3f57..c3455498 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -2166,6 +2166,12 @@ static void LoadMobjThinker(actionf_p1 thinker) mobj->player->viewz = mobj->player->mo->z + mobj->player->viewheight; } + if (mobj->type == MT_SKYBOX) + if (mobj->spawnpoint->options & MTF_OBJECTSPECIAL) + skyboxmo[1] = mobj; + else + skyboxmo[0] = mobj; + P_AddThinker(&mobj->thinker); if (diff2 & MD2_WAYPOINTCAP) @@ -2666,7 +2672,7 @@ static void P_NetUnArchiveThinkers(void) { next = currentthinker->next; - if (currentthinker->function.acp1 == (actionf_p1)P_MobjThinker) + if (currentthinker->function.acp1 == (actionf_p1)P_MobjThinker || currentthinker->function.acp1 == (actionf_p1)P_NullPrecipThinker) P_RemoveSavegameMobj((mobj_t *)currentthinker); // item isn't saved, don't remove it else Z_Free(currentthinker); From 3d311eeae3f3404bf4911d68f9aefe9d4da81544 Mon Sep 17 00:00:00 2001 From: Ashnal Date: Sat, 10 Sep 2022 13:30:24 -0400 Subject: [PATCH 02/11] Fix compiler warning --- src/p_saveg.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/p_saveg.c b/src/p_saveg.c index c3455498..a2656a46 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -2167,10 +2167,12 @@ static void LoadMobjThinker(actionf_p1 thinker) } if (mobj->type == MT_SKYBOX) + { if (mobj->spawnpoint->options & MTF_OBJECTSPECIAL) skyboxmo[1] = mobj; else skyboxmo[0] = mobj; + } P_AddThinker(&mobj->thinker); From 078e249d557615bb4fd42719cf8887a1c760785b Mon Sep 17 00:00:00 2001 From: Alam Ed Arias Date: Sat, 24 Sep 2022 15:58:24 +0000 Subject: [PATCH 03/11] CircleCI: Disable NONET builds --- .circleci/config.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e7916074..8e2dcbe5 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -42,15 +42,15 @@ jobs: paths: - /var/cache/apt/archives - checkout - - run: - name: Compile without network support and BLUA - command: make -C src LINUX=1 ERRORMODE=1 -k NONET=1 NO_LUA=1 - - run: - name: wipe build - command: make -C src LINUX=1 cleandep - - run: - name: rebuild depend - command: make -C src LINUX=1 clean + #- run: + # name: Compile without network support and BLUA + # command: make -C src LINUX=1 ERRORMODE=1 -k NONET=1 NO_LUA=1 + #- run: + # name: wipe build + # command: make -C src LINUX=1 cleandep + #- run: + # name: rebuild depend + # command: make -C src LINUX=1 clean - restore_cache: keys: - v1-SRB2-{{ .Branch }}-{{ checksum "objs/Linux/SDL/Release/depend.dep" }} From 984d44bb57a567a6c6e080e446e4ba02313c5fd7 Mon Sep 17 00:00:00 2001 From: Alam Ed Arias Date: Sat, 24 Sep 2022 16:13:27 +0000 Subject: [PATCH 04/11] CircleCI: we need the depend.dep file for the cache --- .circleci/config.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 8e2dcbe5..10ab56a1 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -45,9 +45,9 @@ jobs: #- run: # name: Compile without network support and BLUA # command: make -C src LINUX=1 ERRORMODE=1 -k NONET=1 NO_LUA=1 - #- run: - # name: wipe build - # command: make -C src LINUX=1 cleandep + - run: + name: wipe build + command: make -C src LINUX=1 cleandep #- run: # name: rebuild depend # command: make -C src LINUX=1 clean From 2db57c72e53c29db74743ea44809d62498ccfc81 Mon Sep 17 00:00:00 2001 From: Alam Ed Arias Date: Sat, 24 Sep 2022 16:28:11 +0000 Subject: [PATCH 05/11] CircleCI: run the clean step to build the 32-bit LINUX depend.dep --- .circleci/config.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 10ab56a1..c08a7f6d 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -45,12 +45,12 @@ jobs: #- run: # name: Compile without network support and BLUA # command: make -C src LINUX=1 ERRORMODE=1 -k NONET=1 NO_LUA=1 - - run: - name: wipe build - command: make -C src LINUX=1 cleandep #- run: - # name: rebuild depend - # command: make -C src LINUX=1 clean + # name: wipe build + # command: make -C src LINUX=1 cleandep + - run: + name: rebuild depend + command: make -C src LINUX=1 clean - restore_cache: keys: - v1-SRB2-{{ .Branch }}-{{ checksum "objs/Linux/SDL/Release/depend.dep" }} From c24a0d0a47c92670fe51fcb08fab7b50a55e0966 Mon Sep 17 00:00:00 2001 From: Ashnal Date: Thu, 29 Sep 2022 15:43:50 -0400 Subject: [PATCH 06/11] Remove usage of currentthinker from direct removal It's designed to be referenced from P_RunTHinkers, whjich we aren't doing --- src/p_mobj.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/p_mobj.c b/src/p_mobj.c index befcf04f..b2a9b09b 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -10323,10 +10323,17 @@ void P_RemoveSavegameMobj(mobj_t *mobj) // stop any playing sound S_StopSound(mobj); + R_RemoveMobjInterpolator(mobj); // free block - P_RemoveThinkerDelayed((thinker_t *)mobj); // Call directly here since we are calling P_InitThinkers - R_RemoveMobjInterpolator(mobj); + // Here we use the same code as R_RemoveThinkerDelayed, but without reference counting (we're removing everything so it shouldn't matter) and without touching currentthinker since we aren't in P_RunThinkers + { + thinker_t *thinker = (thinker_t *)mobj; + thinker_t *next = thinker->next; + (next->prev = thinker->prev)->next = next; + R_DestroyLevelInterpolators(thinker); + Z_Free(thinker); + } } static CV_PossibleValue_t respawnitemtime_cons_t[] = {{1, "MIN"}, {300, "MAX"}, {0, NULL}}; From f8d71450e5e6649e36f82448e92626b3bed713fb Mon Sep 17 00:00:00 2001 From: Ashnal Date: Thu, 29 Sep 2022 19:22:53 -0400 Subject: [PATCH 07/11] Unlink non-mobj and non-precip thinkers when loading and freeing Move globalweather to before P_SpawnSpecials so that specials can properly change weather and have it communicated in savegames --- src/p_mobj.c | 1 - src/p_saveg.c | 6 ++++++ src/p_setup.c | 4 ++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/p_mobj.c b/src/p_mobj.c index b2a9b09b..14800481 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -10331,7 +10331,6 @@ void P_RemoveSavegameMobj(mobj_t *mobj) thinker_t *thinker = (thinker_t *)mobj; thinker_t *next = thinker->next; (next->prev = thinker->prev)->next = next; - R_DestroyLevelInterpolators(thinker); Z_Free(thinker); } } diff --git a/src/p_saveg.c b/src/p_saveg.c index a2656a46..75806003 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -2677,7 +2677,11 @@ static void P_NetUnArchiveThinkers(void) if (currentthinker->function.acp1 == (actionf_p1)P_MobjThinker || currentthinker->function.acp1 == (actionf_p1)P_NullPrecipThinker) P_RemoveSavegameMobj((mobj_t *)currentthinker); // item isn't saved, don't remove it else + { + (next->prev = currentthinker->prev)->next = next; + R_DestroyLevelInterpolators(currentthinker); Z_Free(currentthinker); + } } // we don't want the removed mobjs to come back @@ -3090,6 +3094,7 @@ static inline void P_NetArchiveSpecials(void) // Sky number WRITEINT32(save_p, globallevelskynum); + CONS_Printf("globalweather write %d", globalweather); // Current global weather type WRITEUINT8(save_p, globalweather); @@ -3126,6 +3131,7 @@ static void P_NetUnArchiveSpecials(void) P_SetupLevelSky(j, true); globalweather = READUINT8(save_p); + CONS_Printf("globalweather read %d", globalweather); if (globalweather) { diff --git a/src/p_setup.c b/src/p_setup.c index 328a3ff3..4f4140df 100644 --- a/src/p_setup.c +++ b/src/p_setup.c @@ -3124,14 +3124,14 @@ boolean P_SetupLevel(boolean skipprecip) if (!playerstarts[numcoopstarts]) break; + globalweather = mapheaderinfo[gamemap-1]->weather; + // set up world state P_SpawnSpecials(fromnetsave); if (loadprecip) // ugly hack for P_NetUnArchiveMisc (and P_LoadNetGame) P_SpawnPrecipitation(); - globalweather = mapheaderinfo[gamemap-1]->weather; - #ifdef HWRENDER // not win32 only 19990829 by Kin if (rendermode != render_soft && rendermode != render_none) { From 78ad817cf1afc6b230927805c300d249c785ce78 Mon Sep 17 00:00:00 2001 From: Ashnal Date: Thu, 29 Sep 2022 19:26:42 -0400 Subject: [PATCH 08/11] Forgot to remove my debug prints --- src/p_saveg.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/p_saveg.c b/src/p_saveg.c index 75806003..e3bc51d0 100644 --- a/src/p_saveg.c +++ b/src/p_saveg.c @@ -3094,7 +3094,6 @@ static inline void P_NetArchiveSpecials(void) // Sky number WRITEINT32(save_p, globallevelskynum); - CONS_Printf("globalweather write %d", globalweather); // Current global weather type WRITEUINT8(save_p, globalweather); @@ -3131,7 +3130,6 @@ static void P_NetUnArchiveSpecials(void) P_SetupLevelSky(j, true); globalweather = READUINT8(save_p); - CONS_Printf("globalweather read %d", globalweather); if (globalweather) { From bd4150b90ee358c2bacbe53fe46781d3b7ed9d86 Mon Sep 17 00:00:00 2001 From: Ashnal Date: Mon, 3 Oct 2022 02:07:36 -0400 Subject: [PATCH 09/11] Fixes an issue where mobjs with shadows would never get freed, due to their reference count getting reset after having their shadows spawned, resulting in a reference count of -1 and the mobj never being freed, or a use-after-free during the shadow's thinker. Also adds some P_SetTargets to P_BlockThingsIterator to fix an inconsistency I noticed while investigating this. --- src/p_maputl.c | 4 ++++ src/p_mobj.c | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/p_maputl.c b/src/p_maputl.c index 760e45c4..17df33a9 100644 --- a/src/p_maputl.c +++ b/src/p_maputl.c @@ -1034,7 +1034,10 @@ boolean P_BlockThingsIterator(INT32 x, INT32 y, boolean (*func)(mobj_t *)) { P_SetTarget(&bnext, mobj->bnext); // We want to note our reference to bnext here incase it is MF_NOTHINK and gets removed! if (!func(mobj)) + { + P_SetTarget(&bnext, NULL); return false; + } if (P_MobjWasRemoved(tmthing) // func just popped our tmthing, cannot continue. || (bnext && P_MobjWasRemoved(bnext))) // func just broke blockmap chain, cannot continue. { @@ -1042,6 +1045,7 @@ boolean P_BlockThingsIterator(INT32 x, INT32 y, boolean (*func)(mobj_t *)) return true; } } + P_SetTarget(&bnext, NULL); return true; } diff --git a/src/p_mobj.c b/src/p_mobj.c index 530dd37d..bfdb5a7f 100644 --- a/src/p_mobj.c +++ b/src/p_mobj.c @@ -9893,6 +9893,9 @@ mobj_t *P_SpawnMobj(fixed_t x, fixed_t y, fixed_t z, mobjtype_t type) break; } + if (!(mobj->flags & MF_NOTHINK)) + P_AddThinker(&mobj->thinker); // Needs to come before the shadow spawn, or else the shadow's reference gets forgotten + switch (mobj->type) { case MT_PLAYER: @@ -9916,9 +9919,6 @@ mobj_t *P_SpawnMobj(fixed_t x, fixed_t y, fixed_t z, mobjtype_t type) break; } - if (!(mobj->flags & MF_NOTHINK)) - P_AddThinker(&mobj->thinker); - // Call action functions when the state is set if (st->action.acp1 && (mobj->flags & MF_RUNSPAWNFUNC)) { From 2473c7d7cc3e1ad32587bce613b30a1a36782eff Mon Sep 17 00:00:00 2001 From: Eidolon Date: Wed, 26 Oct 2022 00:59:35 +0000 Subject: [PATCH 10/11] Merge branch 'mobj-jitter' into 'next' Ensure view interpolates between T-1 to T See merge request KartKrew/Kart-Public!317 --- src/p_tick.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/p_tick.c b/src/p_tick.c index 5386b1db..d70c491d 100644 --- a/src/p_tick.c +++ b/src/p_tick.c @@ -23,6 +23,7 @@ #include "lua_script.h" #include "lua_hook.h" #include "k_kart.h" +#include "r_main.h" #include "r_fps.h" // Object place @@ -811,6 +812,20 @@ void P_Ticker(boolean run) { R_UpdateLevelInterpolators(); R_UpdateViewInterpolation(); + + // Hack: ensure newview is assigned every tic. + // Ensures view interpolation is T-1 to T in poor network conditions + // We need a better way to assign view state decoupled from game logic + for (i = 0; i <= splitscreen; i++) + { + player_t *player = &players[displayplayers[i]]; + BOOL skyVisible = skyVisiblePerPlayer[i]; + if (skyVisible && skyboxmo[0] && cv_skybox.value) + { + R_SkyboxFrame(player); + } + R_SetupFrame(player, (skyboxmo[0] && cv_skybox.value)); + } } P_MapEnd(); From e7c16a13b70d4847b840b3ce00aeb3729de3d44f Mon Sep 17 00:00:00 2001 From: Eidolon Date: Wed, 26 Oct 2022 03:04:51 +0000 Subject: [PATCH 11/11] Merge branch 'splitscreen-hudhook-fix' into 'next' Clear and draw all game hud hook calls to 1 list See merge request KartKrew/Kart-Public!318 (cherry picked from commit 7e17eb8591afa9487fd311a6f111bd2ed42b67e0) 2cffc9b4 Clear and draw all game hud hook calls to 1 list --- src/st_stuff.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/st_stuff.c b/src/st_stuff.c index 4a9accf0..0c91ad71 100644 --- a/src/st_stuff.c +++ b/src/st_stuff.c @@ -1967,12 +1967,10 @@ static void ST_overlayDrawer(void) { if (renderisnewtic) { - LUA_HUD_ClearDrawList(luahuddrawlist_game); LUAh_GameHUD(stplyr, luahuddrawlist_game); } - LUA_HUD_DrawList(luahuddrawlist_game); } -#endif +#endif // HAVE_BLUA // draw level title Tails if (*mapheaderinfo[gamemap-1]->lvlttl != '\0' && !(hu_showscores && (netgame || multiplayer) && !mapreset) @@ -2166,6 +2164,12 @@ void ST_Drawer(void) if (st_overlay) { +#ifdef HAVE_BLUA + if (renderisnewtic) + { + LUA_HUD_ClearDrawList(luahuddrawlist_game); + } +#endif // HAVE_BLUA // No deadview! for (i = 0; i <= splitscreen; i++) { @@ -2173,6 +2177,10 @@ void ST_Drawer(void) ST_overlayDrawer(); } +#ifdef HAVE_BLUA + LUA_HUD_DrawList(luahuddrawlist_game); +#endif // HAVE_BLUA + // draw Midnight Channel's overlay ontop if (mapheaderinfo[gamemap-1]->typeoflevel & TOL_TV) // Very specific Midnight Channel stuff. ST_MayonakaStatic();