Store full Source alongside legacy source_identifier on all sourced commands - #15
Open
ryantaylor wants to merge 1 commit into
Open
Store full Source alongside legacy source_identifier on all sourced commands#15ryantaylor wants to merge 1 commit into
ryantaylor wants to merge 1 commit into
Conversation
…ommands BuildSquad, BuildGlobalUpgrade, UseAbility, and CancelProduction (backed by SourcedPbgid/SourcedAbility/SourcedIndex) previously exposed only a legacy truncated u16 source_identifier and discarded the fully parsed Source immediately after wire parsing, unlike every other sourced command variant. Source::parse always decodes the full source regardless of command type — the truncation was purely a backward compatibility choice for these four types, predating the Source enum. Now these three structs store both fields: source_identifier (u16, for existing consumers) and source (Source, for new code going forward), matching the pattern used elsewhere in the crate (e.g. the CMD_CancelConstruction fix) rather than replacing one with the other.
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
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.
Summary
BuildSquad/BuildGlobalUpgrade(SourcedPbgid),UseAbility(SourcedAbility), andCancelProduction(SourcedIndex) previously exposed only a legacy truncatedu16source_identifier— the fully parsedSourcewas thrown away right after wire parsing (Source::parsealways runs and decodes it in full, regardless of command type; the truncation was purely a backward-compat holdover from before theSourceenum existed).Targeted,SourcePbgid,Sourced,Ability) already exposes the fullSource. This closes that gap.source_identifier: u16(kept for existing consumers) andsource: Source(for new code —Self::sourceis now the recommended accessor), rather than replacing one with the other, per the precedent already set bya2eb90a(CMD_CancelConstruction).CommandPayload::source()now returnsSome(..)forBuildSquad/BuildGlobalUpgrade/UseAbility/CancelProduction, where it previously always returnedNone.SourcedPbgid/SourcedIndex/SourcedAbilityare no longerCopy(onlyClone) sinceSource::Squadsholds aVec<u32>, consistent with every other full-Sourcestruct in the crate.cohlib-rbRuby marshalling to includesourcein the hash for these three payload shapes.legacy_sourced_variants_now_expose_full_source) guarding that these four variants now carry a full source across the fixture corpus.Test plan
cargo test --workspace— all greencargo clippy --workspace -- -D warnings— cleancargo fmt --check— cleancargo build -p cohlib -p cohlib-rb --features cohlib/magnus— clean (Ruby-facing binding compiles)cargo run --release --example vault_regressionagainst the ~9.3k-replay real-world corpus — 0 panics, 0 parse errors, 0 unexpectedUnknownfallbacks (confirms no command in the corpus hits theSource::Squadspanic path for these four newly-exposed types)🤖 Generated with Claude Code