From 1270cd47f3ce6623620188284550148141766e95 Mon Sep 17 00:00:00 2001 From: rfortier Date: Thu, 22 Feb 2024 16:55:47 -0500 Subject: [PATCH 1/3] Remove limit to BehaviorVars on the wire. STR is up against the limit, using 62 of a hardwired 64 boolean behavior vars to be synced. This fix removes the limit before it becomes a barrier. Since "unlimited" will bite us, note the base code has some lengths encoded in 16-bit ints that will bite eventually. Trying to sync even a fraction of that many will cause problems, though. Reimplemented the Boolean long long as a (bit) Vector. This made all vars (bools, ints, floats) vectors, so some code simplification of the wire protocol is accessible. The Booleans are just passed "as is," no attempt to detect which ones changed as the space required to send the delta is more than just sending the bits. Overall impact on wire protocol: sending humanoids will be slightly longer, as the 64-bits available is almost full. But that's longer by a byte (or maybe two since I used String to simplify the code). Everything else sent will be shorter and less overhead. There's no avoiding iterating the bits of the Vector. But the overhead when translating the array of to an array of bits and back is about the same. There was a bunch of extra overhead if Serialization::WriteBool() were to be used, so optimized that part out. Overall, should be better on the wire, and neutral-to-better on CPU. 2nd commit msg: Reworked creating and applying Behavior variable diffs and serializing them. Simpler, easier to read, should usually save bytes on the wire. --- Code/client/Games/References.cpp | 13 +- Code/encoding/Structs/AnimationVariables.cpp | 214 +++++++++---------- Code/encoding/Structs/AnimationVariables.h | 4 +- Code/tests/encoding.cpp | 18 +- 4 files changed, 122 insertions(+), 127 deletions(-) diff --git a/Code/client/Games/References.cpp b/Code/client/Games/References.cpp index 49c3d9b05..20b0d3960 100644 --- a/Code/client/Games/References.cpp +++ b/Code/client/Games/References.cpp @@ -227,10 +227,9 @@ void TESObjectREFR::SaveAnimationVariables(AnimationVariables& aVariables) const if (!pVariableSet) return; - aVariables.Booleans = 0; - - aVariables.Floats.resize(pDescriptor->FloatLookupTable.size()); - aVariables.Integers.resize(pDescriptor->IntegerLookupTable.size()); + aVariables.Booleans.assign(pDescriptor->BooleanLookUpTable.size(), false); + aVariables.Floats.assign(pDescriptor->FloatLookupTable.size(), 0.f); + aVariables.Integers.assign(pDescriptor->IntegerLookupTable.size(), 0); #if TP_FALLOUT4 // TODO: maybe send a var with the variables indicating first or third person? @@ -251,14 +250,14 @@ void TESObjectREFR::SaveAnimationVariables(AnimationVariables& aVariables) const continue; if (pFirstPersonVariables->data[*firstPersonIdx] != 0) - aVariables.Booleans |= (1ull << i); + aVariables.Booleans[i] = true; continue; } #endif if (pVariableSet->data[idx] != 0) - aVariables.Booleans |= (1ull << i); + aVariables.Booleans[i] = true; } for (size_t i = 0; i < pDescriptor->FloatLookupTable.size(); ++i) @@ -361,7 +360,7 @@ void TESObjectREFR::LoadAnimationVariables(const AnimationVariables& aVariables) if (pVariableSet->size > idx) { - pVariableSet->data[idx] = (aVariables.Booleans & (1ull << i)) != 0; + pVariableSet->data[idx] = aVariables.Booleans[i]; } } diff --git a/Code/encoding/Structs/AnimationVariables.cpp b/Code/encoding/Structs/AnimationVariables.cpp index c23bfcf8f..b37d51bec 100644 --- a/Code/encoding/Structs/AnimationVariables.cpp +++ b/Code/encoding/Structs/AnimationVariables.cpp @@ -12,141 +12,123 @@ bool AnimationVariables::operator!=(const AnimationVariables& acRhs) const noexc return !this->operator==(acRhs); } +// std::vector implementation is unspecified, but often packed reasonably. +// The spec does not guarantee contiguous memory, though, so somewhat laborious +// translation needed. Should be better than winding down several layers to +// TiltedPhoques::Serialization::WriteBool, though. +// +void AnimationVariables::VectorBool_to_String(const Vector& bools, TiltedPhoques::String& chars) const +{ + chars.assign((bools.size() + 7) >> 3, 0); + + auto citer = chars.begin(); + auto biter = bools.begin(); + for (uint32_t mask = 1; biter < bools.end(); mask = 1, citer++) + for (; mask < 0x100 && biter < bools.end(); mask <<= 1) + *citer |= *biter++ ? mask : 0; +} + +// The Vector must be the correct size when called. +// +void AnimationVariables::String_to_VectorBool(const TiltedPhoques::String& chars, Vector& bools) +{ + bools.assign(bools.size(), false); + + auto citer = chars.begin(); + auto biter = bools.begin(); + for (uint32_t mask = 1; biter < bools.end(); mask = 1, citer++) + for (; mask < 0x100 && biter < bools.end(); mask <<= 1) + *biter++ = (*citer & mask) ? true : false; +} + + void AnimationVariables::Load(std::istream& aInput) { - aInput.read(reinterpret_cast(&Booleans), sizeof(Booleans)); + // Booleans are bitpacked and a bit different, not guaranteed contiguous. + TiltedPhoques::String chars((Booleans.size() + 7) >> 3, 0); + + aInput.read(reinterpret_cast(chars.data()), chars.size()); + String_to_VectorBool(chars, Booleans); aInput.read(reinterpret_cast(Integers.data()), Integers.size() * sizeof(uint32_t)); aInput.read(reinterpret_cast(Floats.data()), Floats.size() * sizeof(float)); } void AnimationVariables::Save(std::ostream& aOutput) const { - aOutput.write(reinterpret_cast(&Booleans), sizeof(Booleans)); + // Booleans bitpacked and not guaranteed contiguous. + TiltedPhoques::String chars; + VectorBool_to_String(Booleans, chars); + + aOutput.write(reinterpret_cast(chars.data()), chars.size()); aOutput.write(reinterpret_cast(Integers.data()), Integers.size() * sizeof(uint32_t)); aOutput.write(reinterpret_cast(Floats.data()), Floats.size() * sizeof(float)); } +// Wire format description. +// +// Sends 3 VarInts, the count of Booleans, Integers and Floats, in that order. Then sends a bitstream of the +// sum of those counts. For the Booleans, these represent the bit values for the Booleans. For the Integers and +// Floats, it represents a truth table for whether the value has changed. If values HAVE changed, they follow on +// the stream. +// +// void AnimationVariables::GenerateDiff(const AnimationVariables& aPrevious, TiltedPhoques::Buffer::Writer& aWriter) const { - uint64_t changes = 0; - uint32_t idx = 0; - - if (Booleans != aPrevious.Booleans) - { - changes |= (1ull << idx); - } - ++idx; - - auto integers = aPrevious.Integers; - if (integers.empty()) - integers.assign(Integers.size(), 0); - - for (auto i = 0u; i < Integers.size(); ++i) - { - if (Integers[i] != integers[i]) - { - changes |= (1ull << idx); - } - ++idx; - } - - auto floats = aPrevious.Floats; - if (floats.empty()) - floats.assign(Floats.size(), 0.f); - - for (auto i = 0u; i < Floats.size(); ++i) - { - if (Floats[i] != floats[i]) - { - changes |= (1ull << idx); - } - ++idx; - } - + const size_t sizeChangedVector = Booleans.size() + Integers.size() + Floats.size(); + auto changedVector = Booleans; + changedVector.reserve(sizeChangedVector); + + for (size_t i = 0; i < Integers.size(); i++) + changedVector.push_back(aPrevious.Integers.size() != Integers.size() || aPrevious.Integers[i] != Integers[i]); + for (size_t i = 0; i < Floats.size(); i++) + changedVector.push_back(aPrevious.Floats.size() != Floats.size() || aPrevious.Floats[i] != Floats[i]); + + // Now serialize: VarInts Booleans.size(), Integers.size(), Floats.size(), + // then the change table bits, then changed Integers, then changed Floats. + TiltedPhoques::Serialization::WriteVarInt(aWriter, Booleans.size()); TiltedPhoques::Serialization::WriteVarInt(aWriter, Integers.size()); TiltedPhoques::Serialization::WriteVarInt(aWriter, Floats.size()); - const auto cDiffBitCount = 1 + Integers.size() + Floats.size(); - - aWriter.WriteBits(changes, cDiffBitCount); - - idx = 0; - if (changes & (1ull << idx)) - { - aWriter.WriteBits(Booleans, 64); - } - ++idx; - - for (const auto value : Integers) - { - if (changes & (1ull << idx)) - { - TiltedPhoques::Serialization::WriteVarInt(aWriter, value & 0xFFFFFFFF); - } - ++idx; - } - - for (const auto value : Floats) - { - if (changes & (1ull << idx)) - { - aWriter.WriteBits(*reinterpret_cast(&value), 32); - } - ++idx; - } + TiltedPhoques::String chars; + VectorBool_to_String(changedVector, chars); + TiltedPhoques::Serialization::WriteString(aWriter, chars); + + auto biter = changedVector.begin() + Booleans.size(); + for (size_t i = 0; i < Integers.size(); i++) + if (*biter++) + TiltedPhoques::Serialization::WriteVarInt(aWriter, Integers[i]); + for (size_t i = 0; i < Floats.size(); i++) + if (*biter++) + TiltedPhoques::Serialization::WriteFloat(aWriter, Floats[i]); } +// Reads 3 VarInts that represent the size of the Booleans, Integers and Floats. +// That's followed by a bitstream in a string of the Booleans values combined +// with a Changed? truth table for Integers and Floats. +// The Changed? table is scanned and for each true bit, the corresponsing Integer +// or Float is deserialized. +// void AnimationVariables::ApplyDiff(TiltedPhoques::Buffer::Reader& aReader) { - const auto cIntegersSize = TiltedPhoques::Serialization::ReadVarInt(aReader); - if (cIntegersSize > 0xFF) - throw std::runtime_error("Too many integers received !"); - - if (Integers.size() != cIntegersSize) - { - Integers.assign(cIntegersSize, 0); - } - - const auto cFloatsSize = TiltedPhoques::Serialization::ReadVarInt(aReader); - if (cFloatsSize > 0xFF) - throw std::runtime_error("Too many floats received !"); - - if (Floats.size() != cFloatsSize) - { - Floats.assign(cFloatsSize, 0.f); - } - - const auto cDiffBitCount = 1 + Integers.size() + Floats.size(); - - uint64_t changes = 0; - uint32_t idx = 0; - - aReader.ReadBits(changes, cDiffBitCount); - - if (changes & (1ull << idx)) - { - aReader.ReadBits(Booleans, 64); - } - ++idx; - - for (auto& value : Integers) - { - if (changes & (1ull << idx)) - { - value = TiltedPhoques::Serialization::ReadVarInt(aReader) & 0xFFFFFFFF; - } - ++idx; - } - - for (auto& value : Floats) - { - if (changes & (1ull << idx)) - { - uint64_t tmp = 0; - aReader.ReadBits(tmp, 32); - uint32_t data = tmp & 0xFFFFFFFF; - value = *reinterpret_cast(&data); - } - ++idx; - } + size_t booleansSize = TiltedPhoques::Serialization::ReadVarInt(aReader); + size_t integersSize = TiltedPhoques::Serialization::ReadVarInt(aReader); + size_t floatsSize = TiltedPhoques::Serialization::ReadVarInt(aReader); + if (Integers.size() != integersSize) + Integers.assign(integersSize, 0); + if (Floats.size() != floatsSize) + Floats.assign(floatsSize, 0.f); + + TiltedPhoques::Vector changedVector(booleansSize + integersSize + floatsSize); + auto chars = TiltedPhoques::Serialization::ReadString(aReader); + String_to_VectorBool(chars, changedVector); + + Booleans.assign(changedVector.begin(), changedVector.begin() + booleansSize); + + auto biter = changedVector.begin() + booleansSize; + for (size_t i = 0; i < integersSize; i++) + if (*biter++) + Integers[i] = TiltedPhoques::Serialization::ReadVarInt(aReader); + for (size_t i = 0; i < floatsSize; i++) + if (*biter++) + Floats[i] = TiltedPhoques::Serialization::ReadFloat(aReader); } diff --git a/Code/encoding/Structs/AnimationVariables.h b/Code/encoding/Structs/AnimationVariables.h index 23ae912e5..331137139 100644 --- a/Code/encoding/Structs/AnimationVariables.h +++ b/Code/encoding/Structs/AnimationVariables.h @@ -6,7 +6,7 @@ using TiltedPhoques::Vector; struct AnimationVariables { - uint64_t Booleans{0}; + Vector Booleans{}; Vector Integers{}; Vector Floats{}; @@ -18,4 +18,6 @@ struct AnimationVariables void GenerateDiff(const AnimationVariables& aPrevious, TiltedPhoques::Buffer::Writer& aWriter) const; void ApplyDiff(TiltedPhoques::Buffer::Reader& aReader); + void VectorBool_to_String(const Vector& bools, TiltedPhoques::String& chars) const; + void String_to_VectorBool(const TiltedPhoques::String& chars, Vector& bools); }; diff --git a/Code/tests/encoding.cpp b/Code/tests/encoding.cpp index 2570e2afe..2e17f2012 100644 --- a/Code/tests/encoding.cpp +++ b/Code/tests/encoding.cpp @@ -276,7 +276,12 @@ TEST_CASE("Differential structures", "[encoding.differential]") GIVEN("AnimationVariables") { AnimationVariables vars, recvVars; - vars.Booleans = 0x12345678ull; + + vars.Booleans.resize(76); + String testString("\xDE\xAD\xBE\xEF" + "\xDE\xAD\xBE\xEF\x76\xB"); + vars.String_to_VectorBool(testString, vars.Booleans); + vars.Floats.push_back(1.f); vars.Floats.push_back(7.f); vars.Floats.push_back(12.f); @@ -305,7 +310,11 @@ TEST_CASE("Differential structures", "[encoding.differential]") REQUIRE(vars.Integers == recvVars.Integers); } - vars.Booleans = 0x9456123ull; + vars.Booleans.resize(33); + vars.Booleans[16] = false; + vars.Booleans[17] = false; + vars.Booleans[18] = false; + vars.Booleans[19] = false; vars.Floats[3] = 42.f; vars.Integers[0] = 18; vars.Integers[3] = 0; @@ -444,7 +453,10 @@ TEST_CASE("Packets", "[encoding.packets]") auto& move = update.UpdatedMovement; AnimationVariables vars; - vars.Booleans = 0x12345678ull; + vars.Booleans.resize(76); + String testString("\xDE\xAD\xBE\xEF\x76\xB"); + vars.String_to_VectorBool(testString, vars.Booleans); + vars.Floats.push_back(1.f); vars.Floats.push_back(7.f); vars.Floats.push_back(12.f); From 1dc68577281b7ba2ceb2aa217762c8e24a0ac650 Mon Sep 17 00:00:00 2001 From: rfortier Date: Sun, 5 May 2024 15:19:56 -0400 Subject: [PATCH 2/3] A bit more space efficiency for negative integers. --- Code/encoding/Structs/AnimationVariables.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/encoding/Structs/AnimationVariables.cpp b/Code/encoding/Structs/AnimationVariables.cpp index b37d51bec..b968f4de1 100644 --- a/Code/encoding/Structs/AnimationVariables.cpp +++ b/Code/encoding/Structs/AnimationVariables.cpp @@ -96,7 +96,7 @@ void AnimationVariables::GenerateDiff(const AnimationVariables& aPrevious, Tilte auto biter = changedVector.begin() + Booleans.size(); for (size_t i = 0; i < Integers.size(); i++) if (*biter++) - TiltedPhoques::Serialization::WriteVarInt(aWriter, Integers[i]); + TiltedPhoques::Serialization::WriteVarInt(aWriter, Integers[i] & 0xFFFFFFFF); for (size_t i = 0; i < Floats.size(); i++) if (*biter++) TiltedPhoques::Serialization::WriteFloat(aWriter, Floats[i]); From cb983743b5468fb9bf08f61f7601337fbb57e1a1 Mon Sep 17 00:00:00 2001 From: rfortier Date: Wed, 8 May 2024 14:00:45 -0400 Subject: [PATCH 3/3] Fixes a potential out-of-bounds reference to aVariables.Booleans now that it's a Vector; initial load-in animation variables might be empty. --- Code/client/Games/References.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/client/Games/References.cpp b/Code/client/Games/References.cpp index 20b0d3960..c319018ee 100644 --- a/Code/client/Games/References.cpp +++ b/Code/client/Games/References.cpp @@ -360,7 +360,7 @@ void TESObjectREFR::LoadAnimationVariables(const AnimationVariables& aVariables) if (pVariableSet->size > idx) { - pVariableSet->data[idx] = aVariables.Booleans[i]; + pVariableSet->data[idx] = aVariables.Booleans.size() > i ? aVariables.Booleans[i] : false; } }