feat(client): a build that says which commit it is, and notices a mixed folder - #264
Merged
Conversation
…ed folder The X-89/X-90 spawn fix ships in Ironfront_Data/Managed/Ironfront.Net.Unity.Server.dll, not in Assembly-CSharp.dll -- which is where somebody looking for "the game code" reaches first. A host who copies the file they expect to matter keeps the old placement code and the old symptom, and nothing anywhere says so. This makes that visible. WHAT IT DOES. tools/build-player.ps1 rewrites two tracked source files with the commit, the UTC build time and a dirty flag immediately before invoking Unity, and restores them in a finally. NetServerBootstrap.Awake logs the server assembly's stamp beside the Shared one; NetClientBootstrap.Awake logs the Shared one, after the dedicated-server guard so one process never prints two build lines. COMPILED IN, NOT WRITTEN BESIDE THE EXE. A build-stamp.txt in the output folder reports the identity of the FOLDER, so against a single stale DLL dropped into a current folder it reads "current" while the code is old -- it would lie in exactly the case it exists for. A field inside the assembly cannot be separated from the code it describes. The cost is that a tracked file holds the value for the length of one build; the finally, and two EditMode tests asserting a checkout reads "dev", are what keep that cost from becoming a stale committed SHA nobody notices. static readonly, NOT const, AND THE DETECTOR DIES WITHOUT IT. C# inlines a const at every use site including ones in other assemblies, so Server reading a const from Shared would bake the value into Server.dll at ITS compile time -- and a stale Server.dll would then compare its own baked copy against its own stamp, find them equal, and report no mismatch. That is the one case the comparison exists for. A static readonly field is read from the defining assembly at RUN time, so ServerBuildStamp sees what the Shared DLL on disk actually says. NOT ConnectRequestPayload.ProtocolVersion. That byte answers "can these two speak to each other", and two builds with identical wire formats but different gameplay code share it -- exactly the X-90 case, where the spawn arithmetic changed and nothing on the wire did. A field carried over the wire also cannot work on its first deployment for a second reason: it is informative only once BOTH ends send it, and the machine under suspicion is by definition the one running the older build that does not. WHAT IT CANNOT DO, stated rather than left to be discovered. It says nothing about a build cut before it shipped -- those report "dev" and are indistinguishable from an Editor build. For that first exchange the procedure is the only instrument, and that is the next commit. The build script checks EVERY field on its own rather than comparing whole texts. A whole-text check only notices all three substitutions failing together: rename one field and the other two still change, the file differs, the check passes, and the build ships a stamp that is half real -- a binary claiming a build time for a commit it does not name. Mutation-tested: renaming any one of the three fields refuses, and so does turning Commit back into a const, which is the regression that would silently kill the mixed-folder comparison. The unmutated source passes. A tree with no git degrades to "dev" with a warning, because a source drop is a legitimate way to build. Verified: stamp replacement exercised against both files, all three fields in both classes rewritten; .meta gate clean at 1951 assets. Four EditMode tests guard the CHECKOUT rather than a build: they assert both files still read "dev". What can go wrong here, and would go wrong silently, is a stamped value getting committed because a build was interrupted between the rewrite and the restore -- after which every later build ships that stale commit as its identity, which is worse than having no stamp at all. They are input-integrity guards, not pinned baselines: they fire on a bad commit, never on a fix, so they owe no companion. MUTATION-TESTED, and the result is discriminating rather than merely red. Setting the SHARED Commit to a fake SHA fails exactly three of the four -- SharedStamp_IsDevelopment_InACheckout, DevelopmentAssemblies_DoNotReportAMismatch and Describe_NamesTheScriptThatWouldHaveStampedIt -- while ServerStamp_IsDevelopment_InACheckout stays GREEN, because the server file was not the one mutated. The suite names which assembly is wrong, not just that something is. 138 of 138 on the unmutated tree; 135 of 138 under the mutation.
…ne it is The procedure half of the build stamp. The stamp can only speak for builds cut after it shipped; for every exchange before that, and for the folder-level mistakes no assembly can see, this is the instrument. Three things it fixes, all observed rather than imagined. Copying one DLL, because the assembly whose name sounds like the game is not the one server fixes land in -- the X-89/X-90 placement work was entirely inside Ironfront.Net.Unity.Server.dll and a host who replaced Assembly-CSharp.dll would have seen the identical symptom with nothing to explain it. Unzipping over an old folder, which is the same failure one level up: whatever the new build no longer produces survives, and the result was never tested as a whole. And judging a build by Ironfront.exe's timestamp, which a green build routinely leaves untouched. Every DLL name, the Player.log path and both script names were read off the tree rather than recalled: the Managed listing comes from build/windows, and the log path is SteelRaven7/Ravenfield because ProjectSettings still carries the base game's identity -- stated in the page so it does not read as a mistake. It is a new page rather than a section of operations.md: that runbook is the Azure VM, containers and the master server, and play-lan.ps1 / playtest-local.ps1 were documented nowhere at all.
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.
The follow-on to #263, and the reason it needs one. Both defects that PR closed live
in
Ironfront_Data/Managed/Ironfront.Net.Unity.Server.dll— not inAssembly-CSharp.dll, which is where somebody told "the spawn bug is fixed"reasonably reaches first. A host who copies the file they expect to matter keeps
every line of the old placement code and sees the identical symptom, and nothing
anywhere says so.
What it does
tools/build-player.ps1rewrites two tracked source files with the commit, the UTCbuild time and a dirty flag immediately before invoking Unity, and restores them in
a
finally. The server logs its own assembly's stamp beside the Shared one; aclient logs the Shared one. When the two disagree, the server logs an error:
Three decisions with reasons, not preferences
Compiled into the assembly, not written beside the exe. A
build-stamp.txtinthe output folder reports the identity of the folder, so against a single stale
DLL dropped into a current folder it reads "current" while the code is old — it
would lie in exactly the case it exists for.
static readonly, notconst— the detector dies without it. C# inlines aconstat every use site including ones in other assemblies. Server reading aconstfrom Shared would bake the value intoServer.dllat its own compile time,so a stale
Server.dllwould compare its own baked copy against its own stamp, findthem equal, and report no mismatch — the one case the comparison exists for. The
build script's patterns require the word
readonly, so turning it back into aconstis refused rather than silently accepted.Not
ConnectRequestPayload.ProtocolVersion. That byte answers "can these twospeak to each other", and two builds with identical wire formats but different
gameplay code share it — exactly the X-90 case. A wire field also cannot work on its
first deployment: it is informative only once both ends send it, and the machine
under suspicion is the one running the older build that does not.
What it cannot do
It says nothing about a build cut before it shipped — those report
devand areindistinguishable from an Editor build.
docs/handing-over-a-build.mdis theinstrument for that first exchange, and for the folder-level mistakes no assembly
can see (unzipping over an old build, judging a build by
Ironfront.exe'stimestamp, which a green build routinely leaves untouched).
Verification
ServerStamp_IsDevelopment_InACheckoutcorrectly stays greenCommitrestored to aconst.metagateThe per-field guard replaced a whole-text comparison that would only have noticed
all three substitutions failing together — rename one field and the build ships a
stamp claiming a build time for a commit it does not name.