February 2015 seam - #211
Merged
Merged
Conversation
Thirty-three merge commits carry no unique content. Four commits touch nothing we have: three patch FMOD version compatibility, where our sound is OpenAL, and one edits the software renderer's plane code.
Ports uzdoom@dc6b45804 and adapts uzdoom@8e1b1aa20. Scrolling up through a run of disabled menu entries jumps several items at once, which stepped straight past the equality test and scrolled the cursor out of view. It now clamps instead. SDL's video subsystem is brought up in I_InitGraphics rather than at startup, since initialising it that early conflicts with GUI toolkits on some systems, and joystick init is guarded so the manager is only built if the subsystem came up. SDL_INIT_TIMER stays where it was: upstream moved it into I_InitTimer, which for us exists only in the Cocoa backend.
Ports the settled form of uzdoom@de4097cc7, uzdoom@1e0a1466a and uzdoom@291861bf5. An inventory item carrying its own inventory now hands that inventory to whoever picks it up. Upstream landed this three times: first in AddInventory with debug output left in, then moved to CallTryPickup still with the debug output, then stripped. Only the end state is here. The flag moves from upstream's 1<<24 to 1<<25, since 1<<24 is Zandronum's IF_FORCERESPAWNINSURVIVAL. ItemFlags is never serialised, so no existing value shifts.
Ports uzdoom@5164b78c3. APathFollower computed hold times from gametic, which counts from engine start and never resets, so a path holding across a level change compared against a number that had run away. HoldTime is serialised, which is how the stale value survived into the next level.
Ports uzdoom@60b735dc6 and uzdoom@c4f932022. A perpetual swing polyobj can never reach the completion check below it, so its early return guarded nothing. listsoundchannels prints every non-evicted channel with its computed position and a total, which is useful when tracking down sounds that will not stop. Also records uzdoom@ec5817869 as already present: both the NUL-to-space sweep over Dehacked patches and the (int)len cast in C_DoCommand are here.
Adapts uzdoom@322742d4b. The commit is a 28-file hygiene pass, and upstream reverted most of it within days -- 7789975b6 undid the info, tarray, decallib and d_net changes, fd7ed2bc2 undid am_map, c_cvars, configfile and more. Both the am_map out-of-bounds guard and the FName const& signatures are gone by the end of the cluster, so porting it whole would have meant landing work upstream had already deleted. Four of the surviving hunks are real defects rather than tidying: SkipChunk advanced its local pointer variable instead of the caller's stream, so skipping a chunk left the read position where it started. FileReader's default constructor never initialised FilePos. SetCompatibilityParams indexed CompatParams before testing the bound, reading one past the end on the last iteration. G_ProcessIFFDemo accepted a demo with no header at all. The unused-variable removals in the same surviving set are cosmetic and left alone.
Adapts uzdoom@b37a98689. A spawned actor can now inherit the caller's sprite and frame. Upstream also respelled the whole SIXF_ enum from 1<<n to hex in the same commit; not taken, since the values are identical and rewriting 24 lines to change nothing only invites a transcription error. The FAF_NODISTFACTOR parenthesis fix it carries is moot here, because the commits that delete that branch entirely were pulled forward during the January seam. The transferred sprite and frame do not reach clients. SpawnThing carries only position, class and net ID, and the post-spawn broadcasts we already do have no sprite/frame equivalent -- SetThingFrame sends a state, not a raw sprite/frame pair. A client draws the spawned actor with its own default sprite. Closing that needs a new command and is its own job; the tracker row records it. Also records uzdoom@7789975b6 as already matching: it reverts sanity-crusade changes we never took, and each hunk was checked rather than assumed.
Adapts uzdoom@2c06987f6. FDynamicBuffer::SetData assigned the local len parameter instead of m_Len when handed a NULL data pointer, so the buffer kept reporting its old length and still looked like it held its old contents. The bug came from the base -- upstream had it too. The same commit also added M_Free(m_Data) in that branch, which is not taken. It leaves m_Data dangling while m_BufferLen keeps its old value, so the next call with a small length skips the realloc and memcpys into freed memory. Upstream HEAD has only the m_Len reset, having removed the free again.
Adapts uzdoom@c4b742ebf. As with part one of the sanity crusade, most of it was reverted by upstream days later, so this takes the state after the revert rather than the commit as written. P_SpawnMissile dereferenced source unconditionally, and a DECORATE caller can reach it with none. The revert keeps the guard and tidies it to source == NULL, which is the form used here. secplane_t floorplane no longer copies from floorsector when 3D floors are compiled in, since P_FindFloorPlane overwrote it on the next line. The AIMREFLECT collapse in the same commit is already present as a ternary from the reflection cluster, and the remaining hunks are unused-variable removals.
Ports uzdoom@003817f41. The alt HUD listed every ammo type any slotted weapon could ever use, which is a long list on a mod with many weapons. hud_showammo now selects between the ready weapon's ammo only, ammo for weapons actually held, or the previous behaviour, which stays the default.
Ports uzdoom@bd93ce63a, adding the AltHUDAmmo option values and the menu entry for the CVAR added a commit earlier. Also records three commits in the FDynamicBuffer::SetData churn as already settled here. Upstream added M_Free, then patched the dangling pointer it created, then removed the free altogether; we declined the free when it first appeared, so we have been at the end state throughout. The SXF_TRANSFERSPRITEFRAME typo fix likewise never applied, since the constant was written with the correct prefix when it was ported.
uzdoom@2d7592c2c puts back the locals the sanity crusade removed from the nodebuilder. We never took those removals, so the tree already matches. Checked each variable rather than the commit title: SplitSeg's dx and dy, ptect around the mprotect call, and bestj in CloseSubsector, which lives in nodebuild_extract.cpp here rather than nodebuild_gl.cpp.
Ports uzdoom@711ac7791, uzdoom@c3227729e and uzdoom@79791629e. FilterCoopRespawnInventory tested defitem != NULL when deciding whether to destroy armour, which is backwards: the else branch then read defitem->SavePercent precisely when defitem was NULL, so respawning in coop as a class with no default armour dereferenced null. FxGlobalVariable::Resolve was missing a break, so the float, fixed and angle cases fell through into the object and class cases and overwrote ValueType. DoGiveInv had an if/else whose branches were identical.
Ports uzdoom@fd7ed2bc2, the large revert of the sanity crusade. Every hunk of the revert already matched here, since we took only the crusade's surviving fixes rather than the commit as written. But reading it caught an under-port of my own: uzdoom@c4b742ebf added null source guards to six P_SpawnMissile functions and only one had been taken. The other five now have them, in the settled form the revert normalises to. That is the second time this cluster has repaid reading the follow-up rather than trusting the original commit.
Ports uzdoom@6423b2fec, uzdoom@867bfd275 and adapts uzdoom@3d7934f1a. A Dehacked text replacement only altered the first occurrence of the source string. The search now repeats until no match remains, poisoning each matched entry so neither the next pass nor a later replacement finds it again, and it no longer sits behind !good since a string can appear in both the tables searched earlier and the general text list. Net_CheckLastRecieved indexed nettics and remoteresend -- both node arrays -- with Net_Arbitrator, which is a player number, and made the mirror-image mistake when looking up the player. Both now go through nodeforplayer. The rename to Net_CheckLastReceived in the same commit is not taken; the misspelling is Zandronum's too, and renaming a function our netcode calls is churn against the fork for no gain.
Ports uzdoom@c6cb8493e and adapts uzdoom@962fc64d2. Both CVARs already existed here with no way to reach them from the menu. The earthquake intensity slider from the same commit was already present.
Ports uzdoom@a3bdbff05, uzdoom@5fc6ff130 and uzdoom@c0eb39ec7. The 3D midtex opening was computed from the texture's own scale alone, so a sidedef carrying its own Y scale got both the height and the offset divisor wrong. This comes with the GetScaledHeight overload the fix needs. A 3D floor with neither ALLSIDES nor BOTHPLANES draws nothing, but was counted as solid when building the sorted lists, clipping the top plane of anything that overlapped it. A_TimeBomb raises the actor 32 units to position the explosion, and without resetting PrevZ the renderer interpolated that jump, smearing the explosion frames upward from where the bomb had been.
Adapts uzdoom@337682934. CF_FLY lived on player_t while MF2_FLY lived on the actor, so travel carried one and not the other -- which is what the sync hack this commit deletes was papering over. The state is now MF7_FLYCHEAT on the actor and the two stay together for free. Upstream numbered the flag 0x00020000, which is MF7_NODECAL here, so it takes the free 0x00080000. flags7 is wire-synced, but only an added bit is involved, so nothing existing shifts. The commit as written tested (flags7 ^= MF7_FLYCHEAT) != 0 in the fly cheat, which is true whenever any other flags7 bit is set; this takes the form upstream settled on. Three call sites we have and upstream does not still set CF_FLY, which nothing reads any more: spectators, savegame restore and demo playback would all have silently lost flight. All three set the actor flag now, as does the extra all-players branch our LS_SetPlayerProperty carries. This is a netcode improvement rather than a risk: CF_FLY was never broadcast, and flags7 is.
Adapts uzdoom@7db035abb, uzdoom@c2e155bb9 and uzdoom@93c12cf25 as one settled change. Floatbobbing items carried +NOGRAVITY purely to reproduce Hexen's placement, which also needed per-map setthingz hacks in compatibility.txt. The flag is gone from 30 actors, the Hexen MAP04 block is gone, and the behaviour is restored properly instead: BCOMPATF_FLOATBOB, set automatically for maps with an original Hexen MAPINFO, holds a map-placed floatbob item at its authored height through special1 rather than letting it settle to the floor. ArtiBoostMana keeps its +NOGRAVITY, as upstream left it -- a blanket removal would have taken 31 rather than 30. special1 is a plain int while mthing->z is our strong fixed_t, so the store and read go through Raw and FromRaw where upstream converted implicitly.
Upstream ships the lump as menudef.z; ours is menudef.txt, so the provenance cross-check has no shared basename to match on.
Both were declined earlier on my own judgement rather than upstream's. The SIXF_ flags now use upstream's hex spelling, values unchanged, and Net_CheckLastRecieved is renamed to Net_CheckLastReceived across d_net.cpp and d_net.h. Neither is netcode behaviour, so upstream is the reference.
Adapts the eleven-commit quake cluster to its settled state at uzdoom@2827c13d0. Porting the intermediate steps would have been wasted work: QuakePower goes to doubles and back to fixed_t, and QF_SCALEUP is removed and re-added, within the same week. Quakes now have separate X, Y and Z intensities plus QF_RELATIVE, SCALEDOWN, SCALEUP, MAX and FULLINTENSITY, exposed to DECORATE as A_QuakeEx. SERVERCOMMANDS_Earthquake carried a single intensity byte, which cannot describe a per-axis quake, so it sends three intensity bytes and a flag word now, with the matching client decode. Worth knowing: SVC_EARTHQUAKE is hand-written rather than generated from protocolspec, so protocol-snapshot does not cover this and reported clean. The [AK] spectator guard in R_SetupFrame and the [BC] broadcast in the DEarthquake constructor are preserved. QuakePower indexes the RNG with the raw 16.16 span and rebuilds the draw through FromRaw, where upstream's plain int did both implicitly. Also ports uzdoom@0f4bca860, SXF_TRANSFERROLL, which is observable here now that roll renders and syncs.
Both bit rehomings in this seam happened for the same reason: upstream takes the next free bit when it adds a flag, and so did Zandronum, so they keep landing on the same one. A dedicated fua_flags field would end that permanently and costs close to nothing -- flagsets are sent one at a time and only when they differ from the default, so an unused field is free on the wire even in a 30k-monster map. Not doing it now because it needs an accurate list of which flags are genuinely ours, and comparing against upstream HEAD is too noisy to trust: upstream has renamed and restructured whole flag families since our base.
Ports uzdoom@fa2e2a852 and uzdoom@2939194ae, adapts uzdoom@3bf24204d. The nuked OPL emulator tested extra low phase bits and decremented phase before flipping the waveform sign, where the flip depends only on which half of the wave the phase falls in. Four call sites, not the one the diff header suggests. The quake savegame guards needed rewriting rather than porting. Upstream guards its new fields at SAVEVER 4519 and 4520, but ours mean different things -- 4519 was DamageMultiply and 4520 TeleFog types -- so upstream's numbers would make a save written by our 4519 build read quake fields it never wrote. Both sets arrive together here behind one guard, and SAVEVER goes to 4522.
Ports uzdoom@173dbd6bc and uzdoom@8e0151b4c. FxFloatCast is the float counterpart to FxIntCast, and frandompick is randompick over float choices. The scanner rule goes in sc_man_scanner.re, not just the token list -- sc_man_scanner.h is generated from it at build time, which is the same trap randompick hit. Also records three quake commits as superseded: they introduced a quakeInfo struct that the rewrite a few hours later discarded, and the compat menu regrouping as a skip, since our CompatibilityOptions carries 61 options to upstream's 36 and its wholesale replacement would delete the 25 that are Zandronum's.
Ports uzdoom@19ab774dc, uzdoom@a64ebc720 and uzdoom@e92f2826e. P_LineAttack can clear linetarget, and A_CStaffCheck dereferenced it immediately afterwards. Both attack blocks are guarded, not just the one the diff header points at; the [CW] client guard and health broadcast inside are preserved. The assert added with frandompick compared against ValueType, which is an FExpressionType object rather than the raw enum. ArtiBoostMana loses its +NOGRAVITY here, which is the actor the earlier floatbob sweep deliberately left alone -- restoring it then and removing it now is the sequential walk working as intended.
Ports uzdoom@5aba252b8, a41af2de7, e4a041cb4 and adapts uzdoom@84f8c299c. Cached GL nodes truncated node coordinates to words, losing precision on large maps; they store the full fixed-point values now and the cache magic goes to ZGL3 so stale caches are rejected rather than misread. WriteLong takes an int, so the coordinates go through Raw where upstream passed the plain fixed_t. SBARINFO's HasWeaponPiece only ever set truth and never cleared it, so a removed piece still read as held. NoDelay calls the spawn state's function directly instead of re-entering SetState and adjusting tics afterwards, and the level banner lower-cases the map name.
Ports uzdoom@c7842a8de, uzdoom@719dfbe5a and adapts uzdoom@5d65b10ae. The saveable-game guards lived in the save CCMD, so anything else reaching G_SaveGame skipped them; they move into G_SaveGame itself, and G_DoSaveGame additionally refuses when not in a level. Upstream tests a multiplayer global Zandronum does not have, so the check here is NETSTATE_SINGLE. The map-name banner ported a commit ago passed an FString where a const char* was wanted, which only breaks off Windows. A missing xinput.h warns rather than failing configuration, since XInput is optional and disables cleanly.
Their content never landed as its own change, only as part of the settled state, so the cited commit shares no file with them.
The move off player_t is a correctness fix -- a cheats bit cannot survive travel -- not a networking gain. Broadcasting flags7 has no spectator exclusion, so a spectator now emits a flag update for a pawn nobody renders. Small and bounded, but a cost rather than an improvement.
rc4l
marked this pull request as ready for review
August 6, 2026 18:06
rc4l
enabled auto-merge
August 6, 2026 18:06
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Walked the February 2015 commits in date order, one at a time. 116 of 116 rows resolved, zero pending.
portedskipadapteddeferredZero
deferred— unlike January's 8, this seam has no VM entanglement anywhere.Real bugs found, none predicted by their titles
FilterCoopRespawnInventorytesteddefitem != NULL; inverted, so the else branch readdefitem->SavePercentexactly when it was NULL.FDynamicBuffer::SetDataassigned the locallenparameter instead ofm_Len, so a cleared buffer kept reporting stale contents. Inherited from the base.A_CStaffCheckdereferencedlinetargetafterP_LineAttack, which can clear it.HasWeaponPieceonly ever set truth, never cleared it, so removed pieces still read as held.Three under-ports caught by reading follow-ups rather than diff headers
c4b742ebfguarded sixP_SpawnMissile*functions and only one had been taken. The OPL waveform fix was four hunks, not the two its header shows. The+NOGRAVITYsweep removed 31 where upstream removed 30 —ArtiBoostManawas deliberately kept, and the walk later reached the commit that removes it.Clusters taken as settled state
The earthquake rewrite (11 rows) — the intermediate steps undo each other:
QuakePowergoes to doubles and back,QF_SCALEUPis removed and re-added. Landing them in order would mean writing code to delete days later. Every row is still recorded individually.The sanity crusade (2 rows, 46 files) — upstream reverted most of it within days, so this takes the state after the reverts: four real defects out of a large hygiene pass.
Netcode
SERVERCOMMANDS_Earthquakesent a single intensity byte and physically could not describe a per-axis quake; it now carries three intensities plus a flag word, with the matching client decode.Worth knowing:
SVC_EARTHQUAKEis hand-written withNetCommandrather than generated fromprotocolspec, soprotocol-snapshotdoes not cover it and reported "unchanged" throughout. That blind spot applies to every hand-written command.Save versions
Upstream's quake guards use its SAVEVER 4519/4520, which mean different things here (DamageMultiply, TeleFog types) — porting them verbatim would have made a save written by our 4519 build read quake fields it never wrote. Rewritten to one guard at our own version; SAVEVER 4521 → 4522.
Verification
ctest1316/1316.commit-tracker-check,commit-tracker-overlap,protocol-snapshot --check,wire-enum-snapshot --checkall clean.