From 95e496eb9fc3d51cb853ef27e8fde4a2825b1075 Mon Sep 17 00:00:00 2001 From: Alejandro Canela Date: Wed, 15 Jul 2026 18:21:29 +0200 Subject: [PATCH 1/4] fix: Fix buckets creation not being read from schema file, update schema validator to check buckets , update tests --- CHANGELOG.md | 6 + benchmarks/bench_writer.cpp | 2 +- docs/FILE_FORMAT.md | 2 + docs/POST_PROCESSING.md | 2 +- docs/SDK_API.md | 7 +- samples/basic_write.cpp | 5 +- .../readers/schema_reader/schema_registry.h | 8 ++ .../readers/schema_reader/schema_validator.h | 5 + sdk/include/vtx/common/vtx_diagnostics.h | 3 + sdk/include/vtx/common/vtx_property_cache.h | 3 + sdk/include/vtx/reader/core/vtx_reader.h | 20 +++ sdk/include/vtx/writer/core/writer.h | 60 ++++++++ .../policies/formatters/bucket_type_sort.h | 71 ++++++++++ .../readers/schema_reader/schema_registry.cpp | 10 ++ .../schema_reader/schema_validator.cpp | 51 +++++++ .../vtx/reader/core/vtx_schema_adapter.cpp | 27 +--- .../formatters/flatbuffers_vtx_policy.cpp | 63 ++------ .../formatters/protobuff_vtx_policy.cpp | 43 +----- tests/common/test_schema_registry.cpp | 69 +++++++++ tests/common/test_schema_validator.cpp | 46 ++++++ tests/writer/test_frame_finalization.cpp | 70 +++++++++ tests/writer/test_roundtrip.cpp | 134 ++++++++++++++++++ 22 files changed, 585 insertions(+), 122 deletions(-) create mode 100644 sdk/include/vtx/writer/policies/formatters/bucket_type_sort.h diff --git a/CHANGELOG.md b/CHANGELOG.md index c9ba0cc..87d8ae8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **writer/api**: one-call **`WriteReplay`** pipeline (#30) -- new header **`sdk/include/vtx/writer/core/vtx_write_replay.h`**. `WriteReplay(config, source, format)` runs the whole recording pipeline in one call: creates the writer (schema missing/invalid -> `SchemaInvalid`), initializes the `IFrameDataSource` (failure -> `Internal`), drains every frame through `TryRecordFrame`, finalizes, and returns a **`WriteReplayResult`** (`ok`, `error`, `warnings`, `frames_written`, `frames_dropped`, `total_frames`, `output_path`, `elapsed_seconds`). Frames rejected by finalization/timer are counted in `frames_dropped` and surfaced one-per-`VtxWarning`; the call still produces a valid replay of the accepted frames. `SerializationFormat` selects FlatBuffers (default) or Protobuf - **writer/api**: automatic output-directory creation + destructor finalize (#29) -- `WriterFacadeConfig::create_output_dirs` (default **on**) creates any missing parent directories of `output_filepath` before the sink opens the file (set `false` to require the directory to pre-exist and keep the previous throw-on-missing behavior). The writer facade destructor now finalizes the file as a best-effort fallback (`Stop()` wrapped in `try/catch`) when the caller forgot to call `Stop()`, so a dropped writer still yields a readable `.vtx` - **writer/api**: in-memory schema sources -- `WriterFacadeConfig` / `NetworkWriterFacadeConfig` (and the internal `ReplayWriter::Config`) gain `schema_json_content` (a raw JSON string) and `schema_registry` (a pre-built `std::shared_ptr`, copied in with no re-parse) alongside `schema_json_path`. Source precedence is **registry > content > path**, applied consistently in the create-time schema probe and in `ReplayWriter`. The writer no longer requires the schema to live on disk +- **common/schema**: the schema's top-level **`"buckets"` array is now parsed and is the source of truth for the frame bucket layout** -- `SchemaRegistry::GetBucketNames()` returns the declared names in order (`"buckets"[i]` names Frame bucket index `i`), and `PropertyAddressCache` carries them (`bucket_names`) so the reader can use them too. A new `Buckets` validation rule rejects a malformed key when present (non-array, non-string entries, empty or duplicate names); schemas without the key stay valid (legacy) +- **writer/api**: **schema-driven bucket normalization** -- before finalization the writer rearranges every frame's buckets into the schema-declared layout: buckets are reordered to schema order, declared buckets missing from the frame are created empty, and a bucket the schema does not declare rejects the frame with the new **`VtxErrorCode::BucketUnresolved`** (observable via `TryRecordFrame`). Frames built positionally (no `bucket_map`) adopt the schema layout as long as they do not exceed the declared bucket count. Schemas without a `"buckets"` array skip the normalization entirely +- **reader/api**: **bucket names restored on read** -- bucket names never hit the wire (both formats serialize buckets positionally), so deserialized frames used to come back with an empty `bucket_map`. The reader now stamps `bucket_map` from the embedded schema's `"buckets"` array when chunks are deserialized, so by-name lookups (`Frame::GetBucket(name)` const) and `bucket_map` iteration work on read frames. Replays written without a `"buckets"` array keep the old positional-only behavior ### Changed @@ -56,6 +59,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **differ**: map fields were never diffed correctly on either backend. The FlatBuffers and Protobuf view adapters' `GetMapSize` / `GetMapKey` / `GetMapValueAsStruct` treated the outer `map_properties` vector (one `MapContainer` per map-field slot) as if each element were a single key/value pair -- so a multi-entry map reported size 1, and added / removed keys were never seen. They now iterate the entries **within** the field's `MapContainer` (slot 0, matching the writer/loader convention), so `DefaultTreeDiff::DiffMapContainers` emits correct add / remove / value-change operations. The Protobuf adapter's "Case 2" also looked for capitalised `Keys` / `Values` fields on the wrong message; it now descends into `map_properties` -> `MapContainer.keys` / `.values`. A single map field per struct is assumed (documented in both adapters) - **writer/reader (Protobuff -> Protobuf rename)** (#28): the misspelled "Protobuff" was removed from the public API -- **breaking, no aliases kept**. `SerializationFormat::Protobuffs` -> **`SerializationFormat::Protobuf`**; `CreateProtobuffWriterFacade` -> **`CreateProtobufWriterFacade`**; `CreateProtobuffNetworkWriterFacade` -> **`CreateProtobufNetworkWriterFacade`**; reader `CreateProtobuffFacade` -> **`CreateProtobufFacade`** (internal `ProtobuffFacadeImpl` -> `ProtobufFacadeImpl`). Migrate call sites with a literal `Protobuff` -> `Protobuf` rename - **common/types**: `FlatArray::GetSubArray` & `FlatArray::GetMutableSubArray` did not have a check for StartIndex being greater or equal to EndIndex. In exceptional cases, like an empty array this can cause a crash or undefined behaviour in the best case scenario. +- **writer/flatbuffers**: `FlatBuffersVtxPolicy::FromNative` hardcoded exactly two bucket slots -- `buckets[0]` was renamed to `"data"`, `buckets[1]` to `"bone_data"`, and **any bucket at index >= 2 was silently dropped** from the file. It now iterates all buckets generically (same shape as the Protobuf policy, shared `Serialization::SortBucketByTypeId` helper in `bucket_type_sort.h`), preserving every bucket and the frame's `bucket_map`. Bucket naming is schema-driven (see the `"buckets"` entries above), not serializer-driven +- **reader/schema**: the reader-side `PropertyAddressCache` built from the embedded schema (`PopulateCacheFromJsonString`) hand-copied the registry cache and **omitted `type_max_indices`**, so `ValidateEntity` on read frames treated every per-type max as 0 (any populated property would flag `FieldIndexOutOfRange` once frame validation ran). It now reuses `SchemaRegistry::GetPropertyCache()` verbatim. Also makes `ValidateReplay`'s per-frame pass effective: it iterates `frame.bucket_map`, which was always empty on read frames before bucket-name restoration +- **samples/benchmarks**: `basic_write.cpp` and `bench_writer.cpp` created a `"Players"` bucket while writing against the arena schema, which declares `"buckets": ["entity"]` -- under schema-driven normalization those frames would now be rejected. Both use `"entity"` ### Notes diff --git a/benchmarks/bench_writer.cpp b/benchmarks/bench_writer.cpp index 9eac4d4..39c43a9 100644 --- a/benchmarks/bench_writer.cpp +++ b/benchmarks/bench_writer.cpp @@ -62,7 +62,7 @@ static void BM_WriterThroughput(benchmark::State& state) { for (int i = 0; i < kFramesPerIteration; ++i) { VTX::Frame frame; - auto& bucket = frame.CreateBucket("Players"); + auto& bucket = frame.CreateBucket("entity"); VTX::PropertyContainer entity; entity.entity_type_id = 0; diff --git a/docs/FILE_FORMAT.md b/docs/FILE_FORMAT.md index a9554f9..ed14edc 100644 --- a/docs/FILE_FORMAT.md +++ b/docs/FILE_FORMAT.md @@ -145,6 +145,8 @@ Frame Bucket names are user-defined and typically represent logical categories in the game world (e.g., `"Players"`, `"Vehicles"`, `"World"`, `"Projectiles"`). A frame can have any number of buckets. +Bucket names never hit the wire: both formats serialize buckets as an ordered, nameless list. The schema's top-level `"buckets"` array is the source of truth for the layout -- `"buckets"[i]` names the bucket stored at index `i`. The writer normalizes every frame to that layout before serialization (rejecting buckets the schema does not declare), and the reader stamps the names back onto `bucket_map` when frames are deserialized. Schemas without a `"buckets"` array skip both steps; their frames are purely positional. + ### Bucket A `Bucket` groups entities of related types. It stores two parallel arrays: diff --git a/docs/POST_PROCESSING.md b/docs/POST_PROCESSING.md index ff276f6..3b188b7 100644 --- a/docs/POST_PROCESSING.md +++ b/docs/POST_PROCESSING.md @@ -300,7 +300,7 @@ Gotchas worth knowing: - **`RemoveIf` invalidates `type_ranges`**: the bucket's typed-range index is wiped to zero after a bulk filter. The serializer rebuilds them from `entity_type_id`, so on-disk output is correct; in-memory tooling that reads `type_ranges` after the processor runs must recompute them or rely on `entity_type_id` directly. -- **Writer bucket limits**: the FlatBuffers serializer only persists `buckets[0]` (renamed to `"data"` on disk) and `buckets[1]` (renamed to `"bone_data"`). Additional buckets created by a processor via `view.raw()->CreateBucket(...)` are **silently dropped**. The Protobuf serializer preserves all buckets but applies the type-id reordering only to bucket 0. +- **Buckets come from the schema**: when the schema declares a top-level `"buckets"` array, the writer normalizes every frame to that layout before serialization -- buckets are reordered to schema order, declared buckets missing from the frame are created empty, and a bucket **not** declared in the schema rejects the whole frame with `VtxErrorCode::BucketUnresolved`. A bucket created by a processor via `view.raw()->CreateBucket(...)` must therefore be declared in the schema. Both serializers persist all buckets positionally and apply the type-id reordering only to bucket 0. Schemas without a `"buckets"` array skip the normalization entirely (legacy behavior). ## Error handling diff --git a/docs/SDK_API.md b/docs/SDK_API.md index 3841dc4..8177d96 100644 --- a/docs/SDK_API.md +++ b/docs/SDK_API.md @@ -183,11 +183,13 @@ config.schema_registry = my_shared_registry; // OR a pre-built std::shared Missing parent directories of `output_filepath` are created automatically (`config.create_output_dirs`, default `true`; set it `false` to require the directory to pre-exist). +The schema's top-level `"buckets"` array declares the frame bucket layout (`SchemaRegistry::GetBucketNames()`), e.g. `"buckets": ["World"]`. When declared, every recorded frame is normalized to that layout: buckets are reordered to schema order, declared buckets missing from the frame are created empty, and a bucket the schema does not declare rejects the frame (`VtxErrorCode::BucketUnresolved`). The reader uses the same array to restore bucket names on deserialized frames. Schemas without a `"buckets"` array skip the normalization (legacy behavior). + ### Recording Frames ```cpp VTX::Frame frame; -auto& bucket = frame.CreateBucket("World"); +auto& bucket = frame.CreateBucket("World"); // must be declared in the schema's "buckets" array // Add entities bucket.unique_ids.push_back("player_001"); @@ -219,7 +221,8 @@ if (!r.IsWritten()) { } ``` -Before a frame enters the chunk pipeline the writer **finalizes** it: validates that every +Before a frame enters the chunk pipeline the writer **finalizes** it: normalizes the bucket +layout to the schema's `"buckets"` array (rejecting undeclared buckets), validates that every entity type resolves to a schema struct, recomputes each entity's `content_hash` after all schema fields and post-processor overrides, then **freezes** the frame (any mutation handle a post-processor stashed is revoked). diff --git a/samples/basic_write.cpp b/samples/basic_write.cpp index 206778e..28a9283 100644 --- a/samples/basic_write.cpp +++ b/samples/basic_write.cpp @@ -45,8 +45,9 @@ int main(int argc, char* argv[]) { for (int i = 0; i < frame_count; ++i) { VTX::Frame frame; - // Create a bucket and add one entity per frame. - VTX::Bucket& bucket = frame.CreateBucket("Players"); + // Create the bucket declared by the schema's "buckets" array and add + // one entity per frame. Buckets not declared there reject the frame. + VTX::Bucket& bucket = frame.CreateBucket("entity"); VTX::PropertyContainer entity; entity.entity_type_id = 0; diff --git a/sdk/include/vtx/common/readers/schema_reader/schema_registry.h b/sdk/include/vtx/common/readers/schema_reader/schema_registry.h index 549836d..4942868 100644 --- a/sdk/include/vtx/common/readers/schema_reader/schema_registry.h +++ b/sdk/include/vtx/common/readers/schema_reader/schema_registry.h @@ -90,6 +90,13 @@ namespace VTX { const VTX::PropertyAddressCache& GetPropertyCache() const { return property_cache_; } + /** + * @brief Bucket names declared by the schema's top-level "buckets" array. + * @details Order matters: name at position i corresponds to Frame bucket index i. + * Empty if the schema declares no buckets (legacy schemas). + */ + const std::vector& GetBucketNames() const { return bucket_names_; } + /** * @brief Validate a schema document without loading it. * @details Runs the full SchemaValidator rule set over @p raw_json and @@ -106,6 +113,7 @@ namespace VTX { std::string json_content_; std::unordered_map structs_; ///< Storage map: Struct Name -> Definition. std::unordered_map struct_type_ids_; + std::vector bucket_names_; ///< Declared bucket layout, in schema order. VTX::PropertyAddressCache property_cache_; int32_t current_type_id_ = 0; bool b_is_valid_; ///< Internal validity flag. diff --git a/sdk/include/vtx/common/readers/schema_reader/schema_validator.h b/sdk/include/vtx/common/readers/schema_reader/schema_validator.h index 371ed56..d540f29 100644 --- a/sdk/include/vtx/common/readers/schema_reader/schema_validator.h +++ b/sdk/include/vtx/common/readers/schema_reader/schema_validator.h @@ -51,6 +51,11 @@ namespace VTX { */ struct RawSchema { std::vector structs; + + bool has_buckets = false; ///< Document has a top-level "buckets" key. + bool buckets_is_array = false; ///< That key is an array (valid shape). + std::vector buckets; ///< String entries of "buckets", in order. + int non_string_bucket_entries = 0; ///< Entries of "buckets" that were not strings. }; /** diff --git a/sdk/include/vtx/common/vtx_diagnostics.h b/sdk/include/vtx/common/vtx_diagnostics.h index c4a521e..0cd440a 100644 --- a/sdk/include/vtx/common/vtx_diagnostics.h +++ b/sdk/include/vtx/common/vtx_diagnostics.h @@ -48,6 +48,7 @@ namespace VTX { SchemaMissing, EntityTypeUnresolved, + BucketUnresolved, FieldIndexOutOfRange, TypeMismatch, ContainerMismatch, @@ -80,6 +81,8 @@ namespace VTX { return "SchemaMissing"; case VtxErrorCode::EntityTypeUnresolved: return "EntityTypeUnresolved"; + case VtxErrorCode::BucketUnresolved: + return "BucketUnresolved"; case VtxErrorCode::FieldIndexOutOfRange: return "FieldIndexOutOfRange"; case VtxErrorCode::TypeMismatch: diff --git a/sdk/include/vtx/common/vtx_property_cache.h b/sdk/include/vtx/common/vtx_property_cache.h index 46532d0..6f4fd15 100644 --- a/sdk/include/vtx/common/vtx_property_cache.h +++ b/sdk/include/vtx/common/vtx_property_cache.h @@ -122,9 +122,12 @@ namespace VTX { // Value: The schema cache containing its specific properties std::unordered_map structs; std::unordered_map name_to_id; + // Bucket names from the schema's "buckets" array; position i names Frame bucket index i. + std::vector bucket_names; void Clear() { structs.clear(); name_to_id.clear(); + bucket_names.clear(); } }; } // namespace VTX diff --git a/sdk/include/vtx/reader/core/vtx_reader.h b/sdk/include/vtx/reader/core/vtx_reader.h index 04f58f1..4de4d89 100644 --- a/sdk/include/vtx/reader/core/vtx_reader.h +++ b/sdk/include/vtx/reader/core/vtx_reader.h @@ -665,6 +665,7 @@ namespace VTX { cc.index = idx; SerializerPolicy::ProcessChunkData(idx, compressed_blob, stop_token, cc.native_frames, cc.decompressed_blob, cc.raw_frames_spans); + RestoreBucketNames(cc.native_frames); return cc; } catch (const std::exception& e) { VTX_ERROR("[READER] Chunk {} deserialization failed: {}", idx, e.what()); @@ -675,6 +676,25 @@ namespace VTX { } } + // Bucket names never hit the wire (buckets serialize positionally), so + // freshly deserialized frames carry an empty bucket_map. The schema's + // "buckets" array names bucket index i with its i-th entry; stamp that + // back so by-name lookups and per-bucket validation work on read frames. + // property_address_cache_ is built once in the constructor (ReadHeader) + // before any chunk load starts, so reading it here is thread-safe. + void RestoreBucketNames(std::vector& frames) const { + const auto& bucket_names = property_address_cache_.bucket_names; + if (bucket_names.empty()) + return; + + for (auto& frame : frames) { + const size_t count = std::min(bucket_names.size(), frame.GetBuckets().size()); + for (size_t i = 0; i < count; ++i) { + frame.bucket_map[bucket_names[i]] = i; + } + } + } + private: std::string filepath_; diff --git a/sdk/include/vtx/writer/core/writer.h b/sdk/include/vtx/writer/core/writer.h index 299a12d..e3f8c4c 100644 --- a/sdk/include/vtx/writer/core/writer.h +++ b/sdk/include/vtx/writer/core/writer.h @@ -109,6 +109,12 @@ namespace VTX { view.Freeze(); } + std::string bucket_detail; + if (!NormalizeBucketsToSchema(native_frame, bucket_detail)) { + timer_.Rollback(); + return RecordResult::MadeRejected(VtxErrorCode::BucketUnresolved, std::move(bucket_detail)); + } + std::string validation_detail; if (!FinalizeFrame(native_frame, validation_detail)) { timer_.Rollback(); @@ -220,6 +226,60 @@ namespace VTX { } private: + // Rearranges the frame's buckets into the schema-declared layout so that + // bucket index i always holds the bucket named by the schema's "buckets"[i]. + // Declared buckets missing from the frame are created empty; a bucket the + // schema does not declare rejects the frame. Schemas without a "buckets" + // array leave the frame untouched (legacy behavior). + bool NormalizeBucketsToSchema(VTX::Frame& frame, std::string& out_detail) { + const std::vector& schema_buckets = registry_.GetBucketNames(); + if (schema_buckets.empty()) { + return true; + } + + auto& buckets = frame.GetMutableBuckets(); + + if (frame.bucket_map.empty() && !buckets.empty()) { + // Positionally built frame (no names): adopt the schema layout as-is. + if (buckets.size() > schema_buckets.size()) { + out_detail = "frame has " + std::to_string(buckets.size()) + + " unnamed buckets but the schema declares only " + + std::to_string(schema_buckets.size()); + return false; + } + buckets.resize(schema_buckets.size()); + for (size_t i = 0; i < schema_buckets.size(); ++i) { + frame.bucket_map[schema_buckets[i]] = i; + } + return true; + } + + if (frame.bucket_map.size() != buckets.size()) { + out_detail = "frame contains buckets with no name; cannot map them onto the schema 'buckets' layout"; + return false; + } + + for (const auto& [name, idx] : frame.bucket_map) { + if (std::find(schema_buckets.begin(), schema_buckets.end(), name) == schema_buckets.end()) { + out_detail = "bucket '" + name + "' is not declared in the schema 'buckets' array"; + return false; + } + } + + std::vector ordered(schema_buckets.size()); + std::map ordered_map; + for (size_t i = 0; i < schema_buckets.size(); ++i) { + auto it = frame.bucket_map.find(schema_buckets[i]); + if (it != frame.bucket_map.end()) { + ordered[i] = std::move(buckets[it->second]); + } + ordered_map[schema_buckets[i]] = i; + } + buckets = std::move(ordered); + frame.bucket_map = std::move(ordered_map); + return true; + } + bool FinalizeFrame(VTX::Frame& frame, std::string& out_detail) { for (auto& bucket : frame.GetMutableBuckets()) { for (auto& entity : bucket.entities) { diff --git a/sdk/include/vtx/writer/policies/formatters/bucket_type_sort.h b/sdk/include/vtx/writer/policies/formatters/bucket_type_sort.h new file mode 100644 index 0000000..c759dcb --- /dev/null +++ b/sdk/include/vtx/writer/policies/formatters/bucket_type_sort.h @@ -0,0 +1,71 @@ +/** + * @file bucket_type_sort.h + * @brief Shared bucket reordering used by the formatter policies before serialization. + * + * @author Zenos Interactive + */ +#pragma once + +#include +#include + +#include "vtx/common/vtx_types.h" + +namespace VTX { + namespace Serialization { + + /** + * @brief Copies @p src into @p dst with entities grouped by entity_type_id, + * rebuilding type_ranges so readers can slice entities per type. + * @details Buckets with no entities (or no valid type ids) are copied verbatim. + */ + inline void SortBucketByTypeId(const VTX::Bucket& src, VTX::Bucket& dst) { + const auto& entities = src.entities; + const auto& ids = src.unique_ids; + + if (entities.empty()) { + dst = src; + return; + } + + int32_t max_type = -1; + for (const auto& ent : entities) { + max_type = std::max(ent.entity_type_id, max_type); + } + + if (max_type < 0) { + dst = src; + return; + } + + std::vector> indices_by_type(max_type + 1); + for (size_t i = 0; i < entities.size(); ++i) { + int32_t t_id = entities[i].entity_type_id; + if (t_id >= 0 && t_id <= max_type) { + indices_by_type[t_id].push_back(i); + } + } + + dst.type_ranges.assign(max_type + 1, {0, 0}); + dst.entities.reserve(entities.size()); + dst.unique_ids.reserve(ids.size()); + + int32_t current_index = 0; + + for (int32_t type_id = 0; type_id <= max_type; ++type_id) { + const auto& indices = indices_by_type[type_id]; + dst.type_ranges[type_id].start_index = current_index; + dst.type_ranges[type_id].count = static_cast(indices.size()); + + for (size_t orig_idx : indices) { + dst.entities.push_back(entities[orig_idx]); + if (orig_idx < ids.size()) { + dst.unique_ids.push_back(ids[orig_idx]); + } + current_index++; + } + } + } + + } // namespace Serialization +} // namespace VTX diff --git a/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp b/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp index a9e371d..38dd094 100644 --- a/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp +++ b/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp @@ -70,8 +70,17 @@ bool VTX::SchemaRegistry::LoadFromRawString(const std::string& raw_json) { structs_.clear(); struct_type_ids_.clear(); + bucket_names_.clear(); current_type_id_ = 0; + if (j.contains("buckets") && j["buckets"].is_array()) { + for (const auto& bucket_json : j["buckets"]) { + if (bucket_json.is_string()) { + bucket_names_.push_back(bucket_json.get()); + } + } + } + for (const auto& struct_json : j["property_mapping"]) { std::string struct_name = struct_json.value("struct", ""); if (struct_name.empty()) @@ -143,6 +152,7 @@ bool VTX::SchemaRegistry::LoadFromRawString(const std::string& raw_json) { } property_cache_.Clear(); + property_cache_.bucket_names = bucket_names_; for (const auto& [struct_name, struct_def] : GetDefinitions()) { int32_t type_id = GetStructTypeId(struct_name); if (type_id == -1) { diff --git a/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_validator.cpp b/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_validator.cpp index 1987732..e25db9f 100644 --- a/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_validator.cpp +++ b/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_validator.cpp @@ -46,6 +46,22 @@ namespace VTX { RawSchema ExtractRawSchema(const json& root, SchemaValidationResult& out) { RawSchema schema; + + if (root.contains("buckets")) { + schema.has_buckets = true; + const auto& buckets_json = root["buckets"]; + schema.buckets_is_array = buckets_json.is_array(); + if (schema.buckets_is_array) { + for (const auto& bucket_json : buckets_json) { + if (bucket_json.is_string()) { + schema.buckets.push_back(bucket_json.get()); + } else { + ++schema.non_string_bucket_entries; + } + } + } + } + const auto& mapping = root["property_mapping"]; int struct_index = 0; @@ -270,6 +286,40 @@ namespace VTX { } }; + /// Top-level "buckets" array (when present) must be a well-formed list of + /// unique, non-empty strings. A schema without the key is legal (legacy); + /// the writer simply cannot enforce a bucket layout for it. + class BucketsRule final : public ISchemaValidationRule { + public: + std::string Name() const override { return "Buckets"; } + void Validate(const RawSchema& schema, SchemaValidationResult& out) const override { + if (!schema.has_buckets) { + return; + } + if (!schema.buckets_is_array) { + out.AddError(Name(), "", "", "'buckets' must be an array of strings."); + return; + } + if (schema.non_string_bucket_entries > 0) { + out.AddError(Name(), "", "", + "'buckets' contains " + std::to_string(schema.non_string_bucket_entries) + + " non-string entr(ies)."); + } + + std::unordered_set seen; + for (size_t i = 0; i < schema.buckets.size(); ++i) { + const std::string& name = schema.buckets[i]; + if (name.empty()) { + out.AddError(Name(), "", "", "'buckets' entry #" + std::to_string(i) + " is an empty string."); + continue; + } + if (!seen.insert(name).second) { + out.AddError(Name(), "", "", "duplicate bucket name '" + name + "' in 'buckets'."); + } + } + } + }; + } // namespace // --------------------------------------------------------------------- @@ -286,6 +336,7 @@ namespace VTX { rules.push_back(std::make_unique()); rules.push_back(std::make_unique()); rules.push_back(std::make_unique()); + rules.push_back(std::make_unique()); return rules; } diff --git a/sdk/src/vtx_reader/src/vtx/reader/core/vtx_schema_adapter.cpp b/sdk/src/vtx_reader/src/vtx/reader/core/vtx_schema_adapter.cpp index 8e86fb1..89d13e8 100644 --- a/sdk/src/vtx_reader/src/vtx/reader/core/vtx_schema_adapter.cpp +++ b/sdk/src/vtx_reader/src/vtx/reader/core/vtx_schema_adapter.cpp @@ -20,30 +20,9 @@ namespace VTX { return; } - for (const auto& [struct_name, struct_def] : temp_registry.GetDefinitions()) { - int32_t type_id = temp_registry.GetStructTypeId(struct_name); - if (type_id == -1) { - VTX_WARN("Struct '{}' does not have a TypeID registered in the enum.", struct_name); - continue; - } - cache.name_to_id[struct_name] = type_id; - - auto& struct_cache = cache.structs[type_id]; - struct_cache.name = struct_name; - for (const auto& field : struct_def.fields) { - if (field.type_id != VTX::FieldType::None) { - VTX::PropertyAddress addr; - addr.index = field.index; - addr.type_id = field.type_id; - addr.container_type = field.container_type; - addr.child_type_name = field.struct_type; - struct_cache.properties[field.name] = addr; - struct_cache.names_by_lookup_key[VTX::MakePropertyLookupKey(field.index, field.type_id, - field.container_type)] = field.name; - struct_cache.property_order.push_back(field.name); - } - } - } + // The registry already builds the full cache while loading (including + // type_max_indices and the schema's bucket names) -- reuse it verbatim. + cache = temp_registry.GetPropertyCache(); } // ========================================================================================= // PROTOBUF (cppvtx::PropertySchema) diff --git a/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp b/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp index ddf103c..0cfab45 100644 --- a/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp +++ b/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp @@ -6,6 +6,7 @@ #include "vtx/common/vtx_types.h" #include "vtx/common/readers/schema_reader/schema_registry.h" #include "vtx/writer/serialization/vtx_to_flatbuffer.h" +#include "vtx/writer/policies/formatters/bucket_type_sort.h" #include "vtx/writer/policies/formatters/flatbuffers_vtx_policy.h" std::string VTX::FlatBuffersVtxPolicy::GetMagicBytes() { @@ -16,64 +17,20 @@ std::unique_ptr VTX::FlatBuffersVtxPolicy: auto sorted_frame = std::make_unique(); const auto& native_buckets = native.GetBuckets(); - if (native_buckets.empty()) - return sorted_frame; + sorted_frame->bucket_map = native.bucket_map; + sorted_frame->GetMutableBuckets().resize(native_buckets.size()); - const auto& native_data = native_buckets[0]; - auto& sorted_data = sorted_frame->GetBucket("data"); + for (size_t b_idx = 0; b_idx < native_buckets.size(); ++b_idx) { + const auto& src_bucket = native_buckets[b_idx]; + auto& dst_bucket = sorted_frame->GetBucket(static_cast(b_idx)); - const auto& entities = native_data.entities; - const auto& ids = native_data.unique_ids; - - if (entities.empty()) { - sorted_data = native_data; - return sorted_frame; - } - - int32_t max_type = -1; - for (const auto& ent : entities) { - max_type = std::max(ent.entity_type_id, max_type); - } - - if (max_type < 0) { - sorted_data = native_data; - return sorted_frame; - } - - std::vector> indices_by_type(max_type + 1); - for (size_t i = 0; i < entities.size(); ++i) { - int32_t t_id = entities[i].entity_type_id; - if (t_id >= 0 && t_id <= max_type) { - indices_by_type[t_id].push_back(i); - } - } - - sorted_data.type_ranges.resize(max_type + 1, {0, 0}); - sorted_data.entities.reserve(entities.size()); - sorted_data.unique_ids.reserve(ids.size()); - - int32_t current_index = 0; - - for (int32_t type_id = 0; type_id <= max_type; ++type_id) { - const auto& indices = indices_by_type[type_id]; - - sorted_data.type_ranges[type_id].start_index = current_index; - sorted_data.type_ranges[type_id].count = static_cast(indices.size()); - - for (size_t orig_idx : indices) { - sorted_data.entities.push_back(entities[orig_idx]); - if (orig_idx < ids.size()) { - sorted_data.unique_ids.push_back(ids[orig_idx]); - } - current_index++; + if (b_idx == 0) { + Serialization::SortBucketByTypeId(src_bucket, dst_bucket); + } else { + dst_bucket = src_bucket; } } - if (native_buckets.size() > 1) { - auto& sorted_bones = sorted_frame->GetBucket("bone_data"); - sorted_bones = native_buckets[1]; - } - return sorted_frame; } diff --git a/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp b/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp index 50fe6d3..0f00b69 100644 --- a/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp +++ b/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp @@ -1,5 +1,6 @@ #include +#include "vtx/writer/policies/formatters/bucket_type_sort.h" #include "vtx/writer/policies/formatters/protobuff_vtx_policy.h" #include "vtx/writer/serialization/vtx_to_proto.h" #include "vtx_schema.pb.h" @@ -11,51 +12,15 @@ std::string VTX::ProtobufVtxPolicy::GetMagicBytes() { std::unique_ptr VTX::ProtobufVtxPolicy::FromNative(VTX::Frame&& native) { const auto& native_buckets = native.GetBuckets(); VTX::Frame sorted_native; + sorted_native.bucket_map = native.bucket_map; sorted_native.GetMutableBuckets().resize(native_buckets.size()); for (size_t b_idx = 0; b_idx < native_buckets.size(); ++b_idx) { const auto& src_bucket = native_buckets[b_idx]; auto& dst_bucket = sorted_native.GetBucket(static_cast(b_idx)); - if (b_idx == 0 && !src_bucket.entities.empty()) { - const auto& entities = src_bucket.entities; - const auto& ids = src_bucket.unique_ids; - int32_t max_type = -1; - for (const auto& ent : entities) { - max_type = std::max(ent.entity_type_id, max_type); - } - - if (max_type >= 0 && !entities.empty()) { - std::vector> indices_by_type(max_type + 1); - for (size_t i = 0; i < entities.size(); ++i) { - int32_t t_id = entities[i].entity_type_id; - if (t_id >= 0 && t_id <= max_type) { - indices_by_type[t_id].push_back(i); - } - } - - dst_bucket.type_ranges.assign(max_type + 1, {0, 0}); - dst_bucket.entities.reserve(entities.size()); - dst_bucket.unique_ids.reserve(ids.size()); - - int32_t current_index = 0; - - for (int32_t type_id = 0; type_id <= max_type; ++type_id) { - const auto& indices = indices_by_type[type_id]; - dst_bucket.type_ranges[type_id].start_index = current_index; - dst_bucket.type_ranges[type_id].count = static_cast(indices.size()); - - for (size_t orig_idx : indices) { - dst_bucket.entities.push_back(entities[orig_idx]); - if (orig_idx < ids.size()) { - dst_bucket.unique_ids.push_back(ids[orig_idx]); - } - current_index++; - } - } - } else { - dst_bucket = src_bucket; - } + if (b_idx == 0) { + Serialization::SortBucketByTypeId(src_bucket, dst_bucket); } else { dst_bucket = src_bucket; } diff --git a/tests/common/test_schema_registry.cpp b/tests/common/test_schema_registry.cpp index 7832f7f..da60118 100644 --- a/tests/common/test_schema_registry.cpp +++ b/tests/common/test_schema_registry.cpp @@ -113,3 +113,72 @@ TEST(SchemaRegistry, GetIndexReturnsNonNegativeForKnownField) { ASSERT_TRUE(schema.LoadFromJson(SchemaPath())); EXPECT_GE(schema.GetIndex("Player", "UniqueID"), 0); } + +// --------------------------------------------------------------------------- +// Bucket names ("buckets" array) +// --------------------------------------------------------------------------- + +namespace { + // Minimal valid property_mapping to append after a custom "buckets" value. + constexpr const char* kTinyMapping = R"("property_mapping": [ + { + "struct": "Tiny", + "values": [ + { + "name": "Score", + "structType": "", + "typeId": "Int32", + "keyId": "None", + "containerType": "None", + "meta": { "type": "int32", "keyType": "", "category": "Tiny", + "displayName": "Score", "tooltip": "", + "defaultValue": "0", "version": 1, "fixedArrayDim": 1 } + } + ] + } + ])"; +} // namespace + +TEST(SchemaRegistry, GetBucketNamesParsesDeclaredBuckets) { + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromJson(SchemaPath())); + + ASSERT_EQ(schema.GetBucketNames().size(), 1u); + EXPECT_EQ(schema.GetBucketNames()[0], "entity"); + + // The property cache carries the same names (used by the reader). + EXPECT_EQ(schema.GetPropertyCache().bucket_names, schema.GetBucketNames()); +} + +TEST(SchemaRegistry, GetBucketNamesPreservesDeclarationOrder) { + const std::string raw = + std::string(R"({ "version": "1.0.0", "buckets": ["entity", "bone_data", "economy"], )") + kTinyMapping + "}"; + + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(raw)); + + const auto& names = schema.GetBucketNames(); + ASSERT_EQ(names.size(), 3u); + EXPECT_EQ(names[0], "entity"); + EXPECT_EQ(names[1], "bone_data"); + EXPECT_EQ(names[2], "economy"); +} + +TEST(SchemaRegistry, GetBucketNamesEmptyWhenKeyMissing) { + const std::string raw = std::string(R"({ "version": "1.0.0", )") + kTinyMapping + "}"; + + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(raw)); + EXPECT_TRUE(schema.GetBucketNames().empty()); + EXPECT_TRUE(schema.GetPropertyCache().bucket_names.empty()); +} + +TEST(SchemaRegistry, ReloadReplacesBucketNames) { + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromJson(SchemaPath())); + ASSERT_FALSE(schema.GetBucketNames().empty()); + + const std::string raw = std::string(R"({ "version": "1.0.0", )") + kTinyMapping + "}"; + ASSERT_TRUE(schema.LoadFromRawString(raw)); + EXPECT_TRUE(schema.GetBucketNames().empty()); +} diff --git a/tests/common/test_schema_validator.cpp b/tests/common/test_schema_validator.cpp index de6a318..ea69cf1 100644 --- a/tests/common/test_schema_validator.cpp +++ b/tests/common/test_schema_validator.cpp @@ -243,6 +243,52 @@ TEST(SchemaValidator, AcceptsFixedArrayDimOnArrayField) { EXPECT_TRUE(Validate(SchemaWithFields(field)).IsValid()); } +// =================================================================== +// #11 Top-level "buckets" array +// =================================================================== + +namespace { + // Same single-struct document as SchemaWithFields, but with a custom + // raw JSON value for the "buckets" key. + std::string SchemaWithBuckets(const std::string& buckets_json) { + return R"({"version":"1.0.0","buckets":)" + buckets_json + R"(,"property_mapping":[{"struct":"S","values":[)" + + std::string(kValidField) + R"(]}]})"; + } +} // namespace + +TEST(SchemaValidator, AcceptsSchemaWithoutBucketsKey) { + // Legacy schemas may not declare buckets; that is not an issue. + const std::string raw = + R"({"version":"1.0.0","property_mapping":[{"struct":"S","values":[)" + std::string(kValidField) + R"(]}]})"; + EXPECT_TRUE(Validate(raw).IsValid()); +} + +TEST(SchemaValidator, AcceptsWellFormedBuckets) { + EXPECT_TRUE(Validate(SchemaWithBuckets(R"(["entity","bone_data"])")).IsValid()); +} + +TEST(SchemaValidator, RejectsNonArrayBuckets) { + EXPECT_TRUE(HasRuleError(Validate(SchemaWithBuckets(R"("entity")")), "Buckets")); +} + +TEST(SchemaValidator, RejectsNonStringBucketEntry) { + EXPECT_TRUE(HasRuleError(Validate(SchemaWithBuckets(R"(["entity", 5])")), "Buckets")); +} + +TEST(SchemaValidator, RejectsEmptyBucketName) { + EXPECT_TRUE(HasRuleError(Validate(SchemaWithBuckets(R"(["entity", ""])")), "Buckets")); +} + +TEST(SchemaValidator, RejectsDuplicateBucketName) { + EXPECT_TRUE(HasRuleError(Validate(SchemaWithBuckets(R"(["entity", "entity"])")), "Buckets")); +} + +TEST(SchemaRegistryValidation, LoadRejectsMalformedBuckets) { + VTX::SchemaRegistry registry; + EXPECT_FALSE(registry.LoadFromRawString(SchemaWithBuckets(R"(["entity", "entity"])"))); + EXPECT_FALSE(registry.GetIsValid()); +} + // =================================================================== // SchemaRegistry integration -- strict rejection // =================================================================== diff --git a/tests/writer/test_frame_finalization.cpp b/tests/writer/test_frame_finalization.cpp index 1669510..0733d40 100644 --- a/tests/writer/test_frame_finalization.cpp +++ b/tests/writer/test_frame_finalization.cpp @@ -396,3 +396,73 @@ TEST(FrameFinalization, PipelineReportSeparatesOutcomes) { EXPECT_EQ(report.skipped, 0u); EXPECT_EQ(report.Total(), 5u); } + +// --------------------------------------------------------------------------- +// Schema-driven buckets -- the frame's bucket layout must match the schema's +// "buckets" array (test_schema.json declares ["entity"]). +// --------------------------------------------------------------------------- + +TEST(FrameFinalization, RejectsBucketNotDeclaredInSchema) { + auto writer = VTX::CreateFlatBuffersWriterFacade(MakeConfig(TestUuid())); + ASSERT_NE(writer, nullptr); + + VTX::Frame frame; + auto& bucket = frame.CreateBucket("not_in_schema"); + auto player = MakePlayer("a", 1, 100.0f); + bucket.unique_ids.push_back("a"); + bucket.entities.push_back(std::move(player)); + + const auto r = writer->TryRecordFrame(frame, Increasing(0.0f)); + EXPECT_FALSE(r.IsWritten()); + EXPECT_EQ(r.error.code, VTX::VtxErrorCode::BucketUnresolved); + EXPECT_NE(r.error.message.find("not_in_schema"), std::string::npos); + + // The rejected frame did not consume an index. + auto good = MakeFrameWith({MakePlayer("b", 1, 100.0f)}); + const auto r1 = writer->TryRecordFrame(good, Increasing(1.0f)); + EXPECT_TRUE(r1.IsWritten()); + EXPECT_EQ(r1.frame_index, 0); + + writer->Stop(); +} + +TEST(FrameFinalization, CreatesDeclaredBucketsMissingFromFrame) { + auto writer = VTX::CreateFlatBuffersWriterFacade(MakeConfig(TestUuid(), /*retain_snapshot=*/true)); + ASSERT_NE(writer, nullptr); + + VTX::Frame empty_frame; // no buckets at all + ASSERT_TRUE(writer->TryRecordFrame(empty_frame, Increasing(0.0f)).IsWritten()); + + // Normalization created the declared "entity" bucket at index 0. + const VTX::Frame* snap = writer->GetLastFinalizedFrame(); + ASSERT_NE(snap, nullptr); + ASSERT_EQ(snap->GetBuckets().size(), 1u); + ASSERT_EQ(snap->bucket_map.size(), 1u); + EXPECT_EQ(snap->bucket_map.at("entity"), 0u); + EXPECT_TRUE(snap->GetBuckets()[0].entities.empty()); + + writer->Stop(); +} + +TEST(FrameFinalization, LegacySchemaWithoutBucketsSkipsEnforcement) { + // A schema that declares no "buckets" array keeps the historical behavior: + // any bucket name is accepted untouched. + auto cfg = MakeConfig(TestUuid()); + cfg.schema_json_path.clear(); + cfg.schema_json_content = + R"({"version":"1.0.0","property_mapping":[{"struct":"Player","values":[)" + R"({"name":"Score","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}})" + R"(]}]})"; + + auto writer = VTX::CreateFlatBuffersWriterFacade(cfg); + ASSERT_NE(writer, nullptr); + + VTX::Frame frame; + auto& bucket = frame.CreateBucket("anything_goes"); + auto player = MakePlayer("a", 1, 100.0f); + bucket.unique_ids.push_back("a"); + bucket.entities.push_back(std::move(player)); + + EXPECT_TRUE(writer->TryRecordFrame(frame, Increasing(0.0f)).IsWritten()); + writer->Stop(); +} diff --git a/tests/writer/test_roundtrip.cpp b/tests/writer/test_roundtrip.cpp index cc172a4..e75eaba 100644 --- a/tests/writer/test_roundtrip.cpp +++ b/tests/writer/test_roundtrip.cpp @@ -156,6 +156,140 @@ TEST_P(RoundtripTest, AcceptsHistoricalUtc) { EXPECT_EQ(ctx.reader->GetTotalFrames(), 20); } +// --------------------------------------------------------------------------- +// Multi-bucket frames survive the round-trip on both backends, and bucket +// names come back from the schema's "buckets" array (they are not stored on +// the wire). Regression: the FlatBuffers writer used to hardcode two bucket +// slots ("data"/"bone_data") and silently drop buckets at index >= 2. +// --------------------------------------------------------------------------- + +namespace { + + constexpr const char* kMultiBucketSchema = R"({ + "version": "1.0.0", + "buckets": ["entity", "bone_data", "economy"], + "property_mapping": [ + { + "struct": "Tiny", + "values": [ + { + "name": "Score", + "structType": "", + "typeId": "Int32", + "keyId": "None", + "containerType": "None", + "meta": { "type": "int32", "keyType": "", "category": "Tiny", + "displayName": "Score", "tooltip": "", + "defaultValue": "0", "version": 1, "fixedArrayDim": 1 } + } + ] + } + ] + })"; + + // Buckets are deliberately created in a different order than the schema + // declares them: the writer must normalize the layout to schema order. + VTX::Frame BuildMultiBucketFrame(int frame_index) { + VTX::Frame f; + int bucket_ordinal = 0; + for (const char* name : {"bone_data", "entity", "economy"}) { + auto& bucket = f.CreateBucket(name); + + VTX::PropertyContainer pc; + pc.entity_type_id = 0; + pc.int32_properties = {frame_index, bucket_ordinal}; + + bucket.unique_ids.push_back(std::string(name) + "_0"); + bucket.entities.push_back(std::move(pc)); + ++bucket_ordinal; + } + return f; + } + +} // namespace + +TEST_P(RoundtripTest, MultiBucketFramesSurviveRoundtrip) { + auto cfg = MakeConfig("multibucket", "uuid-rt-multibucket"); + cfg.schema_json_path.clear(); + cfg.schema_json_content = kMultiBucketSchema; + { + auto writer = CreateWriter(cfg); + ASSERT_TRUE(writer); + for (int i = 0; i < 10; ++i) { + auto frame = BuildMultiBucketFrame(i); + VTX::GameTime::GameTimeRegister t; + t.game_time = float(i) / kFps; + writer->RecordFrame(frame, t); + } + writer->Flush(); + writer->Stop(); + } + + auto ctx = VTX::OpenReplayFile(cfg.output_filepath); + ASSERT_TRUE(ctx) << ctx.error; + ASSERT_EQ(ctx.reader->GetTotalFrames(), 10); + + for (int frame_index : {0, 9}) { + const VTX::Frame* f = ctx.reader->GetFrameSync(frame_index); + ASSERT_NE(f, nullptr) << "frame " << frame_index; + + // All three buckets survive (no silent dropping), in schema order. + ASSERT_EQ(f->GetBuckets().size(), 3u); + ASSERT_EQ(f->bucket_map.size(), 3u); + ASSERT_EQ(f->bucket_map.at("entity"), 0u); + ASSERT_EQ(f->bucket_map.at("bone_data"), 1u); + ASSERT_EQ(f->bucket_map.at("economy"), 2u); + + // Content ends up under the schema-declared position regardless of the + // creation order inside the frame ("bone_data" was created first). + // int32[1] carries the creation ordinal: entity=1, bone_data=0, economy=2. + const auto& buckets = f->GetBuckets(); + ASSERT_EQ(buckets[0].entities.size(), 1u); + EXPECT_EQ(buckets[0].unique_ids[0], "entity_0"); + EXPECT_EQ(buckets[0].entities[0].int32_properties[0], frame_index); + EXPECT_EQ(buckets[0].entities[0].int32_properties[1], 1); + + ASSERT_EQ(buckets[1].entities.size(), 1u); + EXPECT_EQ(buckets[1].unique_ids[0], "bone_data_0"); + EXPECT_EQ(buckets[1].entities[0].int32_properties[1], 0); + + ASSERT_EQ(buckets[2].entities.size(), 1u); + EXPECT_EQ(buckets[2].unique_ids[0], "economy_0"); + EXPECT_EQ(buckets[2].entities[0].int32_properties[1], 2); + } +} + +// --------------------------------------------------------------------------- +// Bucket names restored on read for the single-bucket fixture schema too. +// --------------------------------------------------------------------------- + +TEST_P(RoundtripTest, RestoresBucketNamesFromSchemaOnRead) { + auto cfg = MakeConfig("bucket_names", "uuid-rt-bucketnames"); + { + auto writer = CreateWriter(cfg); + ASSERT_TRUE(writer); + for (int i = 0; i < 5; ++i) { + auto frame = BuildFrame(i); + VTX::GameTime::GameTimeRegister t; + t.game_time = float(i) / kFps; + writer->RecordFrame(frame, t); + } + writer->Stop(); + } + + auto ctx = VTX::OpenReplayFile(cfg.output_filepath); + ASSERT_TRUE(ctx) << ctx.error; + + const VTX::Frame* f = ctx.reader->GetFrameSync(0); + ASSERT_NE(f, nullptr); + ASSERT_EQ(f->bucket_map.size(), 1u); // test_schema.json declares ["entity"] + EXPECT_EQ(f->bucket_map.at("entity"), 0u); + + // By-name const lookup works on read frames now. + const VTX::Frame& frame_ref = *f; + EXPECT_EQ(frame_ref.GetBucket("entity").entities.size(), 1u); +} + // --------------------------------------------------------------------------- // Backend instantiation -- produces: // BothBackends/RoundtripTest.PreservesFrameData/FlatBuffers From 365f12b9aebf513fdd921b0fceb45274de1af669 Mon Sep 17 00:00:00 2001 From: Alejandro Canela Date: Thu, 16 Jul 2026 11:12:59 +0200 Subject: [PATCH 2/4] fix: Harden a latent crash due to buckets initialization from buckets schema name, trying to access a bucket when the bucket list is empty --- CHANGELOG.md | 2 +- .../formatters/flatbuffer_reader_policy.cpp | 12 ++- .../formatters/protobuff_reader_policy.cpp | 11 ++- .../formatters/flatbuffers_vtx_policy.cpp | 9 +- .../formatters/protobuff_vtx_policy.cpp | 9 +- tests/writer/test_roundtrip.cpp | 88 +++++++++++++++++++ 6 files changed, 108 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87d8ae8..88d8646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -60,7 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **writer/reader (Protobuff -> Protobuf rename)** (#28): the misspelled "Protobuff" was removed from the public API -- **breaking, no aliases kept**. `SerializationFormat::Protobuffs` -> **`SerializationFormat::Protobuf`**; `CreateProtobuffWriterFacade` -> **`CreateProtobufWriterFacade`**; `CreateProtobuffNetworkWriterFacade` -> **`CreateProtobufNetworkWriterFacade`**; reader `CreateProtobuffFacade` -> **`CreateProtobufFacade`** (internal `ProtobuffFacadeImpl` -> `ProtobufFacadeImpl`). Migrate call sites with a literal `Protobuff` -> `Protobuf` rename - **common/types**: `FlatArray::GetSubArray` & `FlatArray::GetMutableSubArray` did not have a check for StartIndex being greater or equal to EndIndex. In exceptional cases, like an empty array this can cause a crash or undefined behaviour in the best case scenario. - **writer/flatbuffers**: `FlatBuffersVtxPolicy::FromNative` hardcoded exactly two bucket slots -- `buckets[0]` was renamed to `"data"`, `buckets[1]` to `"bone_data"`, and **any bucket at index >= 2 was silently dropped** from the file. It now iterates all buckets generically (same shape as the Protobuf policy, shared `Serialization::SortBucketByTypeId` helper in `bucket_type_sort.h`), preserving every bucket and the frame's `bucket_map`. Bucket naming is schema-driven (see the `"buckets"` entries above), not serializer-driven -- **reader/schema**: the reader-side `PropertyAddressCache` built from the embedded schema (`PopulateCacheFromJsonString`) hand-copied the registry cache and **omitted `type_max_indices`**, so `ValidateEntity` on read frames treated every per-type max as 0 (any populated property would flag `FieldIndexOutOfRange` once frame validation ran). It now reuses `SchemaRegistry::GetPropertyCache()` verbatim. Also makes `ValidateReplay`'s per-frame pass effective: it iterates `frame.bucket_map`, which was always empty on read frames before bucket-name restoration +- **reader/schema**: the reader-side `PropertyAddressCache` built from the embedded schema (`PopulateCacheFromJsonString`) hand-copied the registry cache and **omitted `type_max_indices`**, so `ValidateEntity` on read frames treated every per-type max as 0 (any populated property would flag `FieldIndexOutOfRange` once frame validation ran). It now reuses `SchemaRegistry::GetPropertyCache()` verbatim. This also makes `ValidateReplay`'s per-frame pass effective **for replays whose schema declares a `"buckets"` array** -- it iterates `frame.bucket_map`, which was always empty on read frames before bucket-name restoration. Replays whose schema has no `"buckets"` array still read back with an empty `bucket_map`, so their per-frame pass remains a no-op (unchanged from before) - **samples/benchmarks**: `basic_write.cpp` and `bench_writer.cpp` created a `"Players"` bucket while writing against the arena schema, which declares `"buckets": ["entity"]` -- under schema-driven normalization those frames would now be rejected. Both use `"entity"` diff --git a/sdk/src/vtx_reader/src/vtx/reader/formatters/flatbuffer_reader_policy.cpp b/sdk/src/vtx_reader/src/vtx/reader/formatters/flatbuffer_reader_policy.cpp index c0879d3..d1b6c4f 100644 --- a/sdk/src/vtx_reader/src/vtx/reader/formatters/flatbuffer_reader_policy.cpp +++ b/sdk/src/vtx_reader/src/vtx/reader/formatters/flatbuffer_reader_policy.cpp @@ -53,10 +53,16 @@ void VTX::FlatBuffersReaderPolicy::ProcessChunkData(int chunk_index, const std:: for (size_t i = 0; i < num_frames; ++i) { if (st.stop_requested()) return; - VTX::Serialization::FromFlat(chunk->frames()->Get(i), out_native_frames[i]); + const auto* frame_fb = chunk->frames()->Get(i); + VTX::Serialization::FromFlat(frame_fb, out_native_frames[i]); - //pack for differ - auto* data_obj = chunk->frames()->Get(i)->data()->Get(0); + //pack bucket 0 for the differ; a frame with no buckets contributes an empty span + const auto* frame_buckets = frame_fb->data(); + if (!frame_buckets || frame_buckets->size() == 0) { + frame_sizes.push_back(0); + continue; + } + auto* data_obj = frame_buckets->Get(0); std::unique_ptr dataT(data_obj->UnPack()); fbb.Clear(); diff --git a/sdk/src/vtx_reader/src/vtx/reader/formatters/protobuff_reader_policy.cpp b/sdk/src/vtx_reader/src/vtx/reader/formatters/protobuff_reader_policy.cpp index 2af17ee..e575ce5 100644 --- a/sdk/src/vtx_reader/src/vtx/reader/formatters/protobuff_reader_policy.cpp +++ b/sdk/src/vtx_reader/src/vtx/reader/formatters/protobuff_reader_policy.cpp @@ -42,7 +42,9 @@ void VTX::ProtobufReaderPolicy::ProcessChunkData(int chunk_index, const std::str //precalculate data size size_t total_raw_size = 0; for (int i = 0; i < num_frames; ++i) { - total_raw_size += proto.frames(i).data(0).ByteSizeLong(); + if (proto.frames(i).data_size() > 0) { + total_raw_size += proto.frames(i).data(0).ByteSizeLong(); + } } out_decompressed_blob.reserve(total_raw_size); @@ -55,8 +57,11 @@ void VTX::ProtobufReaderPolicy::ProcessChunkData(int chunk_index, const std::str return; Serialization::FromProto(proto.frames(i), out_native_frames[i]); - // Eextract root for the differ - std::string rawBytes = proto.frames(i).data(0).SerializeAsString(); + // Extract bucket 0 for the differ; a frame with no buckets contributes an empty span + std::string rawBytes; + if (proto.frames(i).data_size() > 0) { + rawBytes = proto.frames(i).data(0).SerializeAsString(); + } out_decompressed_blob.insert(out_decompressed_blob.end(), rawBytes.begin(), rawBytes.end()); frame_sizes.push_back(rawBytes.size()); } diff --git a/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp b/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp index 0cfab45..85fec1b 100644 --- a/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp +++ b/sdk/src/vtx_writer/src/vtx/writer/formatters/flatbuffers_vtx_policy.cpp @@ -21,14 +21,7 @@ std::unique_ptr VTX::FlatBuffersVtxPolicy: sorted_frame->GetMutableBuckets().resize(native_buckets.size()); for (size_t b_idx = 0; b_idx < native_buckets.size(); ++b_idx) { - const auto& src_bucket = native_buckets[b_idx]; - auto& dst_bucket = sorted_frame->GetBucket(static_cast(b_idx)); - - if (b_idx == 0) { - Serialization::SortBucketByTypeId(src_bucket, dst_bucket); - } else { - dst_bucket = src_bucket; - } + Serialization::SortBucketByTypeId(native_buckets[b_idx], sorted_frame->GetBucket(static_cast(b_idx))); } return sorted_frame; diff --git a/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp b/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp index 0f00b69..0e44c6c 100644 --- a/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp +++ b/sdk/src/vtx_writer/src/vtx/writer/formatters/protobuff_vtx_policy.cpp @@ -16,14 +16,7 @@ std::unique_ptr VTX::ProtobufVtxPolicy::FromN sorted_native.GetMutableBuckets().resize(native_buckets.size()); for (size_t b_idx = 0; b_idx < native_buckets.size(); ++b_idx) { - const auto& src_bucket = native_buckets[b_idx]; - auto& dst_bucket = sorted_native.GetBucket(static_cast(b_idx)); - - if (b_idx == 0) { - Serialization::SortBucketByTypeId(src_bucket, dst_bucket); - } else { - dst_bucket = src_bucket; - } + Serialization::SortBucketByTypeId(native_buckets[b_idx], sorted_native.GetBucket(static_cast(b_idx))); } auto proto = std::make_unique(); diff --git a/tests/writer/test_roundtrip.cpp b/tests/writer/test_roundtrip.cpp index e75eaba..8f5881d 100644 --- a/tests/writer/test_roundtrip.cpp +++ b/tests/writer/test_roundtrip.cpp @@ -256,9 +256,97 @@ TEST_P(RoundtripTest, MultiBucketFramesSurviveRoundtrip) { ASSERT_EQ(buckets[2].entities.size(), 1u); EXPECT_EQ(buckets[2].unique_ids[0], "economy_0"); EXPECT_EQ(buckets[2].entities[0].int32_properties[1], 2); + + // type_ranges must be rebuilt for EVERY bucket, not just index 0, so + // typed access works on non-first buckets too (regression: SortBucketByTypeId + // used to run only for bucket 0, leaving others with empty type_ranges). + EXPECT_EQ(buckets[0].GetEntitiesOfType(0).size(), 1u); + EXPECT_EQ(buckets[1].GetEntitiesOfType(0).size(), 1u); + EXPECT_EQ(buckets[2].GetEntitiesOfType(0).size(), 1u); } } +// --------------------------------------------------------------------------- +// A frame built positionally (raw GetMutableBuckets().push_back, empty +// bucket_map) is adopted into the schema layout instead of rejected. +// --------------------------------------------------------------------------- + +TEST_P(RoundtripTest, AdoptsPositionallyBuiltFrameToSchemaLayout) { + auto cfg = MakeConfig("positional", "uuid-rt-positional"); + cfg.schema_json_path.clear(); + cfg.schema_json_content = kMultiBucketSchema; // declares ["entity","bone_data","economy"] + { + auto writer = CreateWriter(cfg); + ASSERT_TRUE(writer); + for (int i = 0; i < 4; ++i) { + VTX::Frame f; + // Build positionally: no CreateBucket, so bucket_map stays empty. + auto& raw = f.GetMutableBuckets(); + raw.emplace_back(); // will map to schema bucket 0 ("entity") + VTX::PropertyContainer pc; + pc.entity_type_id = 0; + pc.int32_properties = {i}; + raw[0].unique_ids.push_back("e_0"); + raw[0].entities.push_back(std::move(pc)); + + VTX::GameTime::GameTimeRegister t; + t.game_time = float(i) / kFps; + const auto r = writer->TryRecordFrame(f, t); + EXPECT_TRUE(r.IsWritten()) << "positional frame " << i << " rejected: " << r.error.message; + } + writer->Stop(); + } + + auto ctx = VTX::OpenReplayFile(cfg.output_filepath); + ASSERT_TRUE(ctx) << ctx.error; + ASSERT_EQ(ctx.reader->GetTotalFrames(), 4); + + const VTX::Frame* f = ctx.reader->GetFrameSync(0); + ASSERT_NE(f, nullptr); + // The single positional bucket was adopted as schema bucket 0 and the two + // remaining declared buckets were materialized empty. + ASSERT_EQ(f->GetBuckets().size(), 3u); + EXPECT_EQ(f->bucket_map.at("entity"), 0u); + ASSERT_EQ(f->GetBuckets()[0].entities.size(), 1u); + EXPECT_EQ(f->GetBuckets()[0].unique_ids[0], "e_0"); + EXPECT_TRUE(f->GetBuckets()[1].entities.empty()); + EXPECT_TRUE(f->GetBuckets()[2].entities.empty()); +} + +// --------------------------------------------------------------------------- +// A schema without a "buckets" array (legacy) may record zero-bucket frames +// (idle/gap ticks). They must round-trip without crashing on load. +// Regression: the reader policies indexed bucket 0 unconditionally. +// --------------------------------------------------------------------------- + +TEST_P(RoundtripTest, ZeroBucketFramesFromSchemalessWriterRoundtrip) { + auto cfg = MakeConfig("zero_bucket", "uuid-rt-zerobucket"); + cfg.schema_json_path.clear(); + cfg.schema_json_content = + R"({"version":"1.0.0","property_mapping":[{"struct":"S","values":[)" + R"({"name":"X","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}})" + R"(]}]})"; + { + auto writer = CreateWriter(cfg); + ASSERT_TRUE(writer); + for (int i = 0; i < 3; ++i) { + VTX::Frame frame; // zero buckets + VTX::GameTime::GameTimeRegister t; + t.game_time = float(i) / kFps; + writer->RecordFrame(frame, t); + } + writer->Stop(); + } + + auto ctx = VTX::OpenReplayFile(cfg.output_filepath); + ASSERT_TRUE(ctx) << ctx.error; + ASSERT_EQ(ctx.reader->GetTotalFrames(), 3); + + const VTX::Frame* f = ctx.reader->GetFrameSync(0); // must not crash + ASSERT_NE(f, nullptr); + EXPECT_TRUE(f->GetBuckets().empty()); +} + // --------------------------------------------------------------------------- // Bucket names restored on read for the single-bucket fixture schema too. // --------------------------------------------------------------------------- From fd682cc5d80ca3ba8fe0d8bc2a9f66ba01dbbd67 Mon Sep 17 00:00:00 2001 From: Alejandro Canela Date: Thu, 16 Jul 2026 12:38:37 +0200 Subject: [PATCH 3/4] feat: resize flat arryas and maps from schema max type --- CHANGELOG.md | 1 + .../readers/frame_reader/binary_loader.h | 4 +- .../readers/frame_reader/flatbuffer_loader.h | 4 +- .../common/readers/frame_reader/loader_base.h | 9 +- .../readers/frame_reader/native_loader.h | 4 +- .../readers/frame_reader/protobuff_loader.h | 4 +- .../readers/schema_reader/game_schema_types.h | 7 ++ sdk/include/vtx/common/vtx_property_cache.h | 2 + sdk/include/vtx/common/vtx_types.h | 6 ++ sdk/include/vtx/common/vtx_types_helpers.h | 64 ++++++++++--- .../readers/schema_reader/schema_registry.cpp | 15 ++- tests/common/test_native_loader.cpp | 90 ++++++++++++++++++ tests/common/test_schema_registry.cpp | 91 +++++++++++++++++++ 13 files changed, 271 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88d8646..bba831d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **common/schema**: the schema's top-level **`"buckets"` array is now parsed and is the source of truth for the frame bucket layout** -- `SchemaRegistry::GetBucketNames()` returns the declared names in order (`"buckets"[i]` names Frame bucket index `i`), and `PropertyAddressCache` carries them (`bucket_names`) so the reader can use them too. A new `Buckets` validation rule rejects a malformed key when present (non-array, non-string entries, empty or duplicate names); schemas without the key stay valid (legacy) - **writer/api**: **schema-driven bucket normalization** -- before finalization the writer rearranges every frame's buckets into the schema-declared layout: buckets are reordered to schema order, declared buckets missing from the frame are created empty, and a bucket the schema does not declare rejects the frame with the new **`VtxErrorCode::BucketUnresolved`** (observable via `TryRecordFrame`). Frames built positionally (no `bucket_map`) adopt the schema layout as long as they do not exceed the declared bucket count. Schemas without a `"buckets"` array skip the normalization entirely - **reader/api**: **bucket names restored on read** -- bucket names never hit the wire (both formats serialize buckets positionally), so deserialized frames used to come back with an empty `bucket_map`. The reader now stamps `bucket_map` from the embedded schema's `"buckets"` array when chunks are deserialized, so by-name lookups (`Frame::GetBucket(name)` const) and `bucket_map` iteration work on read frames. Replays written without a `"buckets"` array keep the old positional-only behavior +- **common/schema**: **array & map pre-sizing** -- extends the existing scalar pre-sizing (`type_max_indices`) so declared *array* and *map* fields are also materialized on load. `SchemaStruct` / `StructSchemaCache` gain `array_max_indices` (max Array-container index per `FieldType`) and `map_max_index` (count of Struct-valued map fields); `Helpers::ResizeContainerToMaxIndices` now pre-creates one empty subarray per declared array field in the matching `FlatArray` (via new `FlatArray::EnsureSubArrayCount`) and pre-sizes `map_properties`. A declared-but-unpopulated array/map field is now present-and-empty instead of absent, symmetric with scalars. Applies wherever scalar pre-sizing already ran (the four frame loaders + `schema_dynamic_loader`); the loader CRTP hook `GetTypeMaxIndices` became `GetStructSizing` (returns the `StructSchemaCache`) ### Changed diff --git a/sdk/include/vtx/common/readers/frame_reader/binary_loader.h b/sdk/include/vtx/common/readers/frame_reader/binary_loader.h index 1af07ac..0790f16 100644 --- a/sdk/include/vtx/common/readers/frame_reader/binary_loader.h +++ b/sdk/include/vtx/common/readers/frame_reader/binary_loader.h @@ -106,12 +106,12 @@ namespace VTX { return &prop_it->second; } - const std::vector* GetTypeMaxIndices(int32_t entity_type_id) const { + const StructSchemaCache* GetStructSizing(int32_t entity_type_id) const { auto struct_it = cache_->structs.find(entity_type_id); if (struct_it == cache_->structs.end()) { return nullptr; } - return &struct_it->second.type_max_indices; + return &struct_it->second; } /** diff --git a/sdk/include/vtx/common/readers/frame_reader/flatbuffer_loader.h b/sdk/include/vtx/common/readers/frame_reader/flatbuffer_loader.h index db0ff4d..6c24f50 100644 --- a/sdk/include/vtx/common/readers/frame_reader/flatbuffer_loader.h +++ b/sdk/include/vtx/common/readers/frame_reader/flatbuffer_loader.h @@ -46,11 +46,11 @@ namespace VTX { return &prop_it->second; } - const std::vector* GetTypeMaxIndices(int32_t entity_type_id) const { + const StructSchemaCache* GetStructSizing(int32_t entity_type_id) const { auto struct_it = cache_->structs.find(entity_type_id); if (struct_it == cache_->structs.end()) return nullptr; - return &struct_it->second.type_max_indices; + return &struct_it->second; } /** diff --git a/sdk/include/vtx/common/readers/frame_reader/loader_base.h b/sdk/include/vtx/common/readers/frame_reader/loader_base.h index 5ea6ae2..8a48f59 100644 --- a/sdk/include/vtx/common/readers/frame_reader/loader_base.h +++ b/sdk/include/vtx/common/readers/frame_reader/loader_base.h @@ -154,15 +154,16 @@ namespace VTX { protected: /** *The Derived loader MUST provide: - *const std::vector* GetTypeMaxIndices(int32_t entity_type_id) const; + *const StructSchemaCache* GetStructSizing(int32_t entity_type_id) const; */ void PrepareContainer(PropertyContainer& dest) { if (dest.entity_type_id < 0) { return; } - const std::vector* type_max_indices = AsDerived().GetTypeMaxIndices(dest.entity_type_id); - if (type_max_indices != nullptr) { - Helpers::ResizeContainerToMaxIndices(dest, *type_max_indices); + const StructSchemaCache* sizing = AsDerived().GetStructSizing(dest.entity_type_id); + if (sizing != nullptr) { + Helpers::ResizeContainerToMaxIndices(dest, sizing->type_max_indices, sizing->array_max_indices, + sizing->map_max_index); } } diff --git a/sdk/include/vtx/common/readers/frame_reader/native_loader.h b/sdk/include/vtx/common/readers/frame_reader/native_loader.h index 46f3d4b..57e0afb 100644 --- a/sdk/include/vtx/common/readers/frame_reader/native_loader.h +++ b/sdk/include/vtx/common/readers/frame_reader/native_loader.h @@ -54,11 +54,11 @@ namespace VTX { return &prop_it->second; } - const std::vector* GetTypeMaxIndices(int32_t entity_type_id) const { + const StructSchemaCache* GetStructSizing(int32_t entity_type_id) const { auto struct_it = cache_->structs.find(entity_type_id); if (struct_it == cache_->structs.end()) return nullptr; - return &struct_it->second.type_max_indices; + return &struct_it->second; } /** diff --git a/sdk/include/vtx/common/readers/frame_reader/protobuff_loader.h b/sdk/include/vtx/common/readers/frame_reader/protobuff_loader.h index a2d4c75..08add49 100644 --- a/sdk/include/vtx/common/readers/frame_reader/protobuff_loader.h +++ b/sdk/include/vtx/common/readers/frame_reader/protobuff_loader.h @@ -47,11 +47,11 @@ namespace VTX { return &prop_it->second; } - const std::vector* GetTypeMaxIndices(int32_t entity_type_id) const { + const StructSchemaCache* GetStructSizing(int32_t entity_type_id) const { auto struct_it = cache_->structs.find(entity_type_id); if (struct_it == cache_->structs.end()) return nullptr; - return &struct_it->second.type_max_indices; + return &struct_it->second; } /** diff --git a/sdk/include/vtx/common/readers/schema_reader/game_schema_types.h b/sdk/include/vtx/common/readers/schema_reader/game_schema_types.h index 4c36337..027056d 100644 --- a/sdk/include/vtx/common/readers/schema_reader/game_schema_types.h +++ b/sdk/include/vtx/common/readers/schema_reader/game_schema_types.h @@ -79,6 +79,13 @@ namespace VTX { */ std::vector type_max_indices; + /** @brief Max Array-container index (+1) per FieldType, i.e. how many array fields of each + * type the struct declares. Used to pre-create empty subarrays in the matching FlatArray. */ + std::vector array_max_indices; + + /** @brief Number of Map-container fields declared (all Struct-valued); pre-sizes map_properties. */ + int32_t map_max_index = 0; + /// @brief Fast lookup map: Property Name -> Field Pointer. std::unordered_map field_map; diff --git a/sdk/include/vtx/common/vtx_property_cache.h b/sdk/include/vtx/common/vtx_property_cache.h index 6f4fd15..bd95cda 100644 --- a/sdk/include/vtx/common/vtx_property_cache.h +++ b/sdk/include/vtx/common/vtx_property_cache.h @@ -85,6 +85,8 @@ namespace VTX { std::unordered_map names_by_lookup_key; std::vector property_order; std::vector type_max_indices; + std::vector array_max_indices; ///< Max Array-container index (+1) per FieldType. + int32_t map_max_index = 0; ///< Number of Map-container fields (pre-sizes map_properties). std::vector GetPropertiesInOrder() const { std::vector ordered_properties; diff --git a/sdk/include/vtx/common/vtx_types.h b/sdk/include/vtx/common/vtx_types.h index 37ff7b0..a2ff5ee 100644 --- a/sdk/include/vtx/common/vtx_types.h +++ b/sdk/include/vtx/common/vtx_types.h @@ -127,6 +127,12 @@ namespace VTX { /** Creates an empty subarray at the end (i.e., a new marker pointing to current Bucket.size()). */ void CreateEmptySubArray() { offsets.push_back(data.size()); } + /** Ensures at least @p Count subarrays exist, appending empty ones as needed (never shrinks). */ + void EnsureSubArrayCount(size_t Count) { + while (offsets.size() < Count) + offsets.push_back(data.size()); + } + /** Appends a subarray with given elements at the end. */ void AppendSubArray(std::span Items) { offsets.push_back(data.size()); diff --git a/sdk/include/vtx/common/vtx_types_helpers.h b/sdk/include/vtx/common/vtx_types_helpers.h index e0e6a57..5bb9804 100644 --- a/sdk/include/vtx/common/vtx_types_helpers.h +++ b/sdk/include/vtx/common/vtx_types_helpers.h @@ -19,40 +19,74 @@ namespace VTX { namespace Helpers { inline void ResizeContainerToMaxIndices(PropertyContainer& container, - const std::vector& type_max_indices) { - auto GetNeeded = [&](FieldType type) -> int32_t { + const std::vector& type_max_indices, + const std::vector& array_max_indices = {}, + int32_t map_max_index = 0) { + auto GetNeeded = [](const std::vector& maxes, FieldType type) -> int32_t { size_t typeIdx = static_cast(type); - return (typeIdx < type_max_indices.size()) ? type_max_indices[typeIdx] : 0; + return (typeIdx < maxes.size()) ? maxes[typeIdx] : 0; }; - if (int32_t n = GetNeeded(FieldType::Bool)) + // --- Scalars: default-initialized slot per declared field. --- + if (int32_t n = GetNeeded(type_max_indices, FieldType::Bool)) container.bool_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Int32)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Int32)) container.int32_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Int64)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Int64)) container.int64_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Float)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Float)) container.float_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Double)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Double)) container.double_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::String)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::String)) container.string_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Vector)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Vector)) container.vector_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Quat)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Quat)) container.quat_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Transform)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Transform)) container.transform_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::FloatRange)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::FloatRange)) container.range_properties.resize(n); - if (int32_t n = GetNeeded(FieldType::Struct)) + if (int32_t n = GetNeeded(type_max_indices, FieldType::Struct)) container.any_struct_properties.resize(n); + + // --- Arrays: one empty subarray per declared array field of each type. --- + if (int32_t n = GetNeeded(array_max_indices, FieldType::Bool)) + container.bool_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::Int32)) + container.int32_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::Int64)) + container.int64_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::Float)) + container.float_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::Double)) + container.double_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::String)) + container.string_arrays.EnsureSubArrayCount(n); + + if (int32_t n = GetNeeded(array_max_indices, FieldType::Vector)) + container.vector_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::Quat)) + container.quat_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::Transform)) + container.transform_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(array_max_indices, FieldType::FloatRange)) + container.range_arrays.EnsureSubArrayCount(n); + + if (int32_t n = GetNeeded(array_max_indices, FieldType::Struct)) + container.any_struct_arrays.EnsureSubArrayCount(n); + + // --- Maps: all Struct-valued, a single contiguous index space. --- + if (map_max_index > 0) + container.map_properties.resize(static_cast(map_max_index)); } inline void PreparePropertyContainer(PropertyContainer& container, const SchemaStruct& schema) { - ResizeContainerToMaxIndices(container, schema.type_max_indices); + ResizeContainerToMaxIndices(container, schema.type_max_indices, schema.array_max_indices, + schema.map_max_index); } inline uint64_t CalculateContainerHash(const PropertyContainer& container) { diff --git a/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp b/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp index 38dd094..b9b200d 100644 --- a/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp +++ b/sdk/src/vtx_common/src/vtx/common/readers/schema_reader/schema_registry.cpp @@ -138,15 +138,22 @@ bool VTX::SchemaRegistry::LoadFromRawString(const std::string& raw_json) { for (const auto& field : current_struct.fields) { current_struct.field_map[field.name] = &field; - if (field.container_type == VTX::FieldContainerType::None) { - size_t typeIdx = static_cast(field.type_id); + const size_t typeIdx = static_cast(field.type_id); + if (field.container_type == VTX::FieldContainerType::None) { if (typeIdx >= current_struct.type_max_indices.size()) { current_struct.type_max_indices.resize(typeIdx + 1, 0); } - current_struct.type_max_indices[typeIdx] = std::max(current_struct.type_max_indices[typeIdx], field.index + 1); + } else if (field.container_type == VTX::FieldContainerType::Array) { + if (typeIdx >= current_struct.array_max_indices.size()) { + current_struct.array_max_indices.resize(typeIdx + 1, 0); + } + current_struct.array_max_indices[typeIdx] = + std::max(current_struct.array_max_indices[typeIdx], field.index + 1); + } else if (field.container_type == VTX::FieldContainerType::Map) { + current_struct.map_max_index = std::max(current_struct.map_max_index, field.index + 1); } } } @@ -165,6 +172,8 @@ bool VTX::SchemaRegistry::LoadFromRawString(const std::string& raw_json) { auto& struct_cache = property_cache_.structs[type_id]; struct_cache.name = struct_name; struct_cache.type_max_indices = struct_def.type_max_indices; + struct_cache.array_max_indices = struct_def.array_max_indices; + struct_cache.map_max_index = struct_def.map_max_index; for (const auto& field : struct_def.fields) { if (field.type_id != VTX::FieldType::None) { diff --git a/tests/common/test_native_loader.cpp b/tests/common/test_native_loader.cpp index 17c5f8e..d73ea53 100644 --- a/tests/common/test_native_loader.cpp +++ b/tests/common/test_native_loader.cpp @@ -88,6 +88,21 @@ namespace vtx_native_loader_test { // Rotation (Quat[0]) and IsAlive (Bool[0]) are not mapped at all. }; + // ----------------------------------------------------------------- + // Setup 4: a struct that maps ONLY scalar fields of a schema that + // also declares array + map fields. Used to prove the loader + // pre-creates the declared-but-unpopulated array subarrays and map + // slots (symmetric with scalar pre-sizing) via PrepareContainer -> + // GetStructSizing -> ResizeContainerToMaxIndices. + // ----------------------------------------------------------------- + + struct HeroScalarOnly { + std::string unique_id; // String[0] (Name = String[1] left unmapped) + int score = 0; // Int32[0] + // Cooldowns (float[]), Tags/Names (string[]), Inventory (struct[]) and + // AmmoByWeapon (struct map) are all declared in the schema but unmapped. + }; + } // namespace vtx_native_loader_test // StructMapping specializations live in namespace VTX (qualified form). @@ -127,10 +142,45 @@ struct VTX::StructMapping { } }; +template <> +struct VTX::StructMapping { + static constexpr auto GetFields() { + using P = vtx_native_loader_test::HeroScalarOnly; + return std::make_tuple(MakeStructField("UniqueID", &P::unique_id), MakeStructField("Score", &P::score)); + } +}; + namespace { std::string SchemaPath() { return VtxTest::FixturePath("test_schema.json"); } + + // "Hero" declares scalar fields plus one float array (Cooldowns), two string + // arrays (Tags, Names), one struct array (Inventory) and one struct map + // (AmmoByWeapon). HeroScalarOnly maps only the scalars, so the array/map + // fields exercise the loader's array/map pre-sizing path. + constexpr const char* kHeroSchema = R"({ + "version": "1.0.0", + "buckets": ["entity"], + "property_mapping": [ + { "struct": "Item", "values": [ + {"name":"Id","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}} + ]}, + { "struct": "Ammo", "values": [ + {"name":"Count","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}} + ]}, + { "struct": "Hero", "values": [ + {"name":"UniqueID","typeId":"String","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":1}}, + {"name":"Name","typeId":"String","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":1}}, + {"name":"Score","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}}, + {"name":"Cooldowns","typeId":"Float","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Tags","typeId":"String","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Names","typeId":"String","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Inventory","typeId":"Struct","containerType":"Array","keyId":"None","structType":"Item","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"AmmoByWeapon","typeId":"Struct","containerType":"Map","keyId":"String","structType":"Ammo","meta":{"defaultValue":"","fixedArrayDim":1}} + ]} + ] + })"; } // namespace // =================================================================== @@ -240,6 +290,46 @@ TEST(NativeLoader, PreSizesContainerToSchemaMaxOnPartialMapping) { EXPECT_NE(dest.content_hash, 0u); } +// The loader pre-creates declared-but-unpopulated ARRAY and MAP fields as +// empty slots, symmetric with scalar pre-sizing. Exercises the full loader glue: +// PrepareContainer -> GetStructSizing -> ResizeContainerToMaxIndices(array/map). +TEST(NativeLoader, PreSizesArraysAndMapsThroughLoader) { + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(kHeroSchema)); + + VTX::GenericNativeLoader loader(schema.GetPropertyCache()); + + vtx_native_loader_test::HeroScalarOnly h {}; + h.unique_id = "hero_1"; + h.score = 7; + + VTX::PropertyContainer dest; + loader.Load(h, dest, "Hero"); + + EXPECT_GE(dest.entity_type_id, 0); + + // Scalars: mapped + pre-sized (UniqueID+Name -> 2 strings; Score -> 1 int32). + ASSERT_EQ(dest.string_properties.size(), 2u); + EXPECT_EQ(dest.string_properties[0], "hero_1"); + ASSERT_EQ(dest.int32_properties.size(), 1u); + EXPECT_EQ(dest.int32_properties[0], 7); + + // Arrays: declared but unpopulated -> present as empty subarrays. + ASSERT_EQ(dest.float_arrays.SubArrayCount(), 1u); // Cooldowns + EXPECT_TRUE(dest.float_arrays.GetSubArray(0).empty()); + ASSERT_EQ(dest.string_arrays.SubArrayCount(), 2u); // Tags, Names + EXPECT_TRUE(dest.string_arrays.GetSubArray(0).empty()); + EXPECT_TRUE(dest.string_arrays.GetSubArray(1).empty()); + ASSERT_EQ(dest.any_struct_arrays.SubArrayCount(), 1u); // Inventory + + // Array types with no declared field stay untouched. + EXPECT_EQ(dest.int32_arrays.SubArrayCount(), 0u); + + // Map: declared but unpopulated -> present as one empty slot. + ASSERT_EQ(dest.map_properties.size(), 1u); // AmmoByWeapon + EXPECT_TRUE(dest.map_properties[0].keys.empty()); +} + TEST(NativeLoader, ADLConversionFromCustomMathType) { VTX::SchemaRegistry schema; ASSERT_TRUE(schema.LoadFromJson(SchemaPath())); diff --git a/tests/common/test_schema_registry.cpp b/tests/common/test_schema_registry.cpp index da60118..56982ca 100644 --- a/tests/common/test_schema_registry.cpp +++ b/tests/common/test_schema_registry.cpp @@ -6,6 +6,8 @@ #include #include "vtx/common/readers/schema_reader/schema_registry.h" +#include "vtx/common/vtx_types.h" +#include "vtx/common/vtx_types_helpers.h" #include "util/test_fixtures.h" @@ -182,3 +184,92 @@ TEST(SchemaRegistry, ReloadReplacesBucketNames) { ASSERT_TRUE(schema.LoadFromRawString(raw)); EXPECT_TRUE(schema.GetBucketNames().empty()); } + +// --------------------------------------------------------------------------- +// Array / Map pre-sizing (symmetric with scalar pre-sizing) +// --------------------------------------------------------------------------- + +namespace { + // Player declares: 2 int32 scalars, 1 float array, 2 string arrays, + // 1 struct array (Inventory of Item) and 1 struct map (AmmoByWeapon of Ammo). + constexpr const char* kArrayMapSchema = R"({ + "version": "1.0.0", + "buckets": ["entity"], + "property_mapping": [ + { "struct": "Item", "values": [ + {"name":"Id","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}} + ]}, + { "struct": "Ammo", "values": [ + {"name":"Count","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}} + ]}, + { "struct": "Player", "values": [ + {"name":"Score","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}}, + {"name":"Level","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}}, + {"name":"Cooldowns","typeId":"Float","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Tags","typeId":"String","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Names","typeId":"String","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Inventory","typeId":"Struct","containerType":"Array","keyId":"None","structType":"Item","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"AmmoByWeapon","typeId":"Struct","containerType":"Map","keyId":"String","structType":"Ammo","meta":{"defaultValue":"","fixedArrayDim":1}} + ]} + ] + })"; +} // namespace + +TEST(SchemaRegistry, PreSizesArraysAndMapsFromSchema) { + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(kArrayMapSchema)); + + const VTX::SchemaStruct* player = schema.GetStruct("Player"); + ASSERT_NE(player, nullptr); + + VTX::PropertyContainer c; + VTX::Helpers::PreparePropertyContainer(c, *player); + + // Scalars: unchanged (two int32 fields). + EXPECT_EQ(c.int32_properties.size(), 2u); + + // Arrays: one empty subarray per declared array field, grouped per type. + EXPECT_EQ(c.float_arrays.SubArrayCount(), 1u); + EXPECT_EQ(c.string_arrays.SubArrayCount(), 2u); + EXPECT_EQ(c.any_struct_arrays.SubArrayCount(), 1u); + EXPECT_TRUE(c.float_arrays.GetSubArray(0).empty()); + EXPECT_TRUE(c.string_arrays.GetSubArray(1).empty()); + + // Array types with no declared fields stay untouched. + EXPECT_EQ(c.int32_arrays.SubArrayCount(), 0u); + EXPECT_EQ(c.vector_arrays.SubArrayCount(), 0u); + + // Map: one empty, Struct-valued map slot. + ASSERT_EQ(c.map_properties.size(), 1u); + EXPECT_TRUE(c.map_properties[0].keys.empty()); + EXPECT_TRUE(c.map_properties[0].values.empty()); +} + +TEST(SchemaRegistry, ArrayAndMapSizingLandInPropertyCache) { + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(kArrayMapSchema)); + + const int32_t player_id = schema.GetStructTypeId("Player"); + ASSERT_GE(player_id, 0); + + const auto& sc = schema.GetPropertyCache().structs.at(player_id); + const auto string_idx = static_cast(VTX::FieldType::String); + const auto float_idx = static_cast(VTX::FieldType::Float); + ASSERT_GT(sc.array_max_indices.size(), string_idx); + EXPECT_EQ(sc.array_max_indices[string_idx], 2); + EXPECT_EQ(sc.array_max_indices[float_idx], 1); + EXPECT_EQ(sc.map_max_index, 1); +} + +// A schema without array/map fields leaves those sizes empty/zero. +TEST(SchemaRegistry, ScalarOnlySchemaHasNoArrayOrMapSizing) { + const std::string raw = std::string(R"({ "version": "1.0.0", )") + kTinyMapping + "}"; + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(raw)); + + const int32_t id = schema.GetStructTypeId("Tiny"); + ASSERT_GE(id, 0); + const auto& sc = schema.GetPropertyCache().structs.at(id); + EXPECT_TRUE(sc.array_max_indices.empty()); + EXPECT_EQ(sc.map_max_index, 0); +} From 7d16c0abbb799550942df4faf2bb844b7ac7fca6 Mon Sep 17 00:00:00 2001 From: Alejandro Canela Date: Thu, 16 Jul 2026 15:33:43 +0200 Subject: [PATCH 4/4] feat: ensure maps and flat arrays get resize using the schema max types --- CHANGELOG.md | 1 + sdk/include/vtx/common/vtx_types_helpers.h | 87 +++++++----- sdk/include/vtx/reader/core/vtx_reader.h | 39 ++++++ tests/common/test_native_loader.cpp | 42 +++++- tests/writer/test_roundtrip.cpp | 156 +++++++++++++++++++++ 5 files changed, 286 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bba831d..a2d2c5d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **writer/api**: **schema-driven bucket normalization** -- before finalization the writer rearranges every frame's buckets into the schema-declared layout: buckets are reordered to schema order, declared buckets missing from the frame are created empty, and a bucket the schema does not declare rejects the frame with the new **`VtxErrorCode::BucketUnresolved`** (observable via `TryRecordFrame`). Frames built positionally (no `bucket_map`) adopt the schema layout as long as they do not exceed the declared bucket count. Schemas without a `"buckets"` array skip the normalization entirely - **reader/api**: **bucket names restored on read** -- bucket names never hit the wire (both formats serialize buckets positionally), so deserialized frames used to come back with an empty `bucket_map`. The reader now stamps `bucket_map` from the embedded schema's `"buckets"` array when chunks are deserialized, so by-name lookups (`Frame::GetBucket(name)` const) and `bucket_map` iteration work on read frames. Replays written without a `"buckets"` array keep the old positional-only behavior - **common/schema**: **array & map pre-sizing** -- extends the existing scalar pre-sizing (`type_max_indices`) so declared *array* and *map* fields are also materialized on load. `SchemaStruct` / `StructSchemaCache` gain `array_max_indices` (max Array-container index per `FieldType`) and `map_max_index` (count of Struct-valued map fields); `Helpers::ResizeContainerToMaxIndices` now pre-creates one empty subarray per declared array field in the matching `FlatArray` (via new `FlatArray::EnsureSubArrayCount`) and pre-sizes `map_properties`. A declared-but-unpopulated array/map field is now present-and-empty instead of absent, symmetric with scalars. Applies wherever scalar pre-sizing already ran (the four frame loaders + `schema_dynamic_loader`); the loader CRTP hook `GetTypeMaxIndices` became `GetStructSizing` (returns the `StructSchemaCache`) +- **reader/api**: **declared-empty arrays restored on read** -- an array with no data is not serialized (nothing to store), so deserialized entities used to come back missing those subarrays even for schema-declared array fields (maps already round-trip their slot count). The reader now re-creates the declared-but-empty subarrays from the embedded schema (`Helpers::EnsureDeclaredArrays`, grow-only, recursing into nested structs / struct-array elements / map values) so a read frame mirrors the array layout of an ingest-loaded frame. Arrays that carry data round-trip unchanged (their `offsets` already encode empty subarrays); scalars and maps are untouched ### Changed diff --git a/sdk/include/vtx/common/vtx_types_helpers.h b/sdk/include/vtx/common/vtx_types_helpers.h index 5bb9804..207888d 100644 --- a/sdk/include/vtx/common/vtx_types_helpers.h +++ b/sdk/include/vtx/common/vtx_types_helpers.h @@ -18,66 +18,79 @@ using int64 = std::int64_t; namespace VTX { namespace Helpers { + // Grow-only: ensures each FlatArray holds at least the schema-declared number of + // (possibly empty) subarrays. Never shrinks; leaves scalars and maps untouched. + // Reused by ResizeContainerToMaxIndices (write/ingest) and by the reader to + // restore declared-but-empty array fields that were not serialized. + inline void EnsureDeclaredArrays(PropertyContainer& container, const std::vector& array_max_indices) { + auto GetNeeded = [&](FieldType type) -> int32_t { + size_t typeIdx = static_cast(type); + return (typeIdx < array_max_indices.size()) ? array_max_indices[typeIdx] : 0; + }; + + if (int32_t n = GetNeeded(FieldType::Bool)) + container.bool_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::Int32)) + container.int32_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::Int64)) + container.int64_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::Float)) + container.float_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::Double)) + container.double_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::String)) + container.string_arrays.EnsureSubArrayCount(n); + + if (int32_t n = GetNeeded(FieldType::Vector)) + container.vector_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::Quat)) + container.quat_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::Transform)) + container.transform_arrays.EnsureSubArrayCount(n); + if (int32_t n = GetNeeded(FieldType::FloatRange)) + container.range_arrays.EnsureSubArrayCount(n); + + if (int32_t n = GetNeeded(FieldType::Struct)) + container.any_struct_arrays.EnsureSubArrayCount(n); + } + inline void ResizeContainerToMaxIndices(PropertyContainer& container, const std::vector& type_max_indices, const std::vector& array_max_indices = {}, int32_t map_max_index = 0) { - auto GetNeeded = [](const std::vector& maxes, FieldType type) -> int32_t { + auto GetNeeded = [&](FieldType type) -> int32_t { size_t typeIdx = static_cast(type); - return (typeIdx < maxes.size()) ? maxes[typeIdx] : 0; + return (typeIdx < type_max_indices.size()) ? type_max_indices[typeIdx] : 0; }; // --- Scalars: default-initialized slot per declared field. --- - if (int32_t n = GetNeeded(type_max_indices, FieldType::Bool)) + if (int32_t n = GetNeeded(FieldType::Bool)) container.bool_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Int32)) + if (int32_t n = GetNeeded(FieldType::Int32)) container.int32_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Int64)) + if (int32_t n = GetNeeded(FieldType::Int64)) container.int64_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Float)) + if (int32_t n = GetNeeded(FieldType::Float)) container.float_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Double)) + if (int32_t n = GetNeeded(FieldType::Double)) container.double_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::String)) + if (int32_t n = GetNeeded(FieldType::String)) container.string_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Vector)) + if (int32_t n = GetNeeded(FieldType::Vector)) container.vector_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Quat)) + if (int32_t n = GetNeeded(FieldType::Quat)) container.quat_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Transform)) + if (int32_t n = GetNeeded(FieldType::Transform)) container.transform_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::FloatRange)) + if (int32_t n = GetNeeded(FieldType::FloatRange)) container.range_properties.resize(n); - if (int32_t n = GetNeeded(type_max_indices, FieldType::Struct)) + if (int32_t n = GetNeeded(FieldType::Struct)) container.any_struct_properties.resize(n); // --- Arrays: one empty subarray per declared array field of each type. --- - if (int32_t n = GetNeeded(array_max_indices, FieldType::Bool)) - container.bool_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::Int32)) - container.int32_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::Int64)) - container.int64_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::Float)) - container.float_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::Double)) - container.double_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::String)) - container.string_arrays.EnsureSubArrayCount(n); - - if (int32_t n = GetNeeded(array_max_indices, FieldType::Vector)) - container.vector_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::Quat)) - container.quat_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::Transform)) - container.transform_arrays.EnsureSubArrayCount(n); - if (int32_t n = GetNeeded(array_max_indices, FieldType::FloatRange)) - container.range_arrays.EnsureSubArrayCount(n); - - if (int32_t n = GetNeeded(array_max_indices, FieldType::Struct)) - container.any_struct_arrays.EnsureSubArrayCount(n); + EnsureDeclaredArrays(container, array_max_indices); // --- Maps: all Struct-valued, a single contiguous index space. --- if (map_max_index > 0) diff --git a/sdk/include/vtx/reader/core/vtx_reader.h b/sdk/include/vtx/reader/core/vtx_reader.h index 4de4d89..69da043 100644 --- a/sdk/include/vtx/reader/core/vtx_reader.h +++ b/sdk/include/vtx/reader/core/vtx_reader.h @@ -19,6 +19,7 @@ #include "vtx_deserializer_service.h" #include "vtx/common/vtx_diagnostics.h" #include "vtx/common/vtx_types.h" +#include "vtx/common/vtx_types_helpers.h" #include "vtx/common/vtx_concepts.h" #include "vtx/reader/core/vtx_schema_adapter.h" #include "vtx/common/vtx_frame_accessor.h" @@ -666,6 +667,7 @@ namespace VTX { SerializerPolicy::ProcessChunkData(idx, compressed_blob, stop_token, cc.native_frames, cc.decompressed_blob, cc.raw_frames_spans); RestoreBucketNames(cc.native_frames); + RestoreDeclaredArrays(cc.native_frames); return cc; } catch (const std::exception& e) { VTX_ERROR("[READER] Chunk {} deserialization failed: {}", idx, e.what()); @@ -695,6 +697,43 @@ namespace VTX { } } + // A declared array field with no data is not serialized (an empty array has + // nothing to store), so deserialized entities come back missing those + // subarrays. Re-create the declared-but-empty subarrays from the schema so a + // read frame mirrors the same array layout an ingest-loaded frame has (maps + // already round-trip their slot count, so only arrays need this). Recurses + // into nested structs / struct-array elements / map values, matching the + // loader's recursive PrepareContainer. Grow-only: never touches populated + // arrays, scalars, or maps. + void RestoreDeclaredArrays(std::vector& frames) const { + if (property_address_cache_.structs.empty()) + return; + for (auto& frame : frames) { + for (auto& bucket : frame.GetMutableBuckets()) { + for (auto& entity : bucket.entities) { + PreSizeDeclaredArraysRecursive(entity); + } + } + } + } + + void PreSizeDeclaredArraysRecursive(VTX::PropertyContainer& container) const { + auto it = property_address_cache_.structs.find(container.entity_type_id); + if (it != property_address_cache_.structs.end()) { + Helpers::EnsureDeclaredArrays(container, it->second.array_max_indices); + } + for (auto& nested : container.any_struct_properties) + PreSizeDeclaredArraysRecursive(nested); + for (auto& nested : container.any_struct_arrays.data) + PreSizeDeclaredArraysRecursive(nested); + for (auto& map_item : container.map_properties) + for (auto& value : map_item.values) + PreSizeDeclaredArraysRecursive(value); + for (auto& map_item : container.map_arrays.data) + for (auto& value : map_item.values) + PreSizeDeclaredArraysRecursive(value); + } + private: std::string filepath_; diff --git a/tests/common/test_native_loader.cpp b/tests/common/test_native_loader.cpp index d73ea53..c6fb069 100644 --- a/tests/common/test_native_loader.cpp +++ b/tests/common/test_native_loader.cpp @@ -11,7 +11,10 @@ #include #include "vtx/common/adapters/native/struct_mapping.h" +#include "vtx/common/readers/frame_reader/binary_loader.h" +#include "vtx/common/readers/frame_reader/flatbuffer_loader.h" #include "vtx/common/readers/frame_reader/native_loader.h" +#include "vtx/common/readers/frame_reader/protobuff_loader.h" #include "vtx/common/readers/schema_reader/schema_registry.h" #include "vtx/common/vtx_property_cache.h" #include "vtx/common/vtx_types.h" @@ -315,9 +318,9 @@ TEST(NativeLoader, PreSizesArraysAndMapsThroughLoader) { EXPECT_EQ(dest.int32_properties[0], 7); // Arrays: declared but unpopulated -> present as empty subarrays. - ASSERT_EQ(dest.float_arrays.SubArrayCount(), 1u); // Cooldowns + ASSERT_EQ(dest.float_arrays.SubArrayCount(), 1u); // Cooldowns EXPECT_TRUE(dest.float_arrays.GetSubArray(0).empty()); - ASSERT_EQ(dest.string_arrays.SubArrayCount(), 2u); // Tags, Names + ASSERT_EQ(dest.string_arrays.SubArrayCount(), 2u); // Tags, Names EXPECT_TRUE(dest.string_arrays.GetSubArray(0).empty()); EXPECT_TRUE(dest.string_arrays.GetSubArray(1).empty()); ASSERT_EQ(dest.any_struct_arrays.SubArrayCount(), 1u); // Inventory @@ -330,6 +333,41 @@ TEST(NativeLoader, PreSizesArraysAndMapsThroughLoader) { EXPECT_TRUE(dest.map_properties[0].keys.empty()); } +// All four frame loaders must expose the same array/map sizing to the shared +// base PrepareContainer via GetStructSizing. PreSizesArraysAndMapsThroughLoader +// (above) pins the base PrepareContainer -> ResizeContainerToMaxIndices path via +// the native loader; this pins each loader's GetStructSizing hook so a break in +// any single loader's hook is caught. +TEST(FrameLoaders, GetStructSizingExposesArrayAndMapSizingForAllLoaders) { + VTX::SchemaRegistry schema; + ASSERT_TRUE(schema.LoadFromRawString(kHeroSchema)); + const auto& cache = schema.GetPropertyCache(); + + const int32_t heroId = schema.GetStructTypeId("Hero"); + ASSERT_GE(heroId, 0); + + const auto floatIdx = static_cast(VTX::FieldType::Float); + const auto stringIdx = static_cast(VTX::FieldType::String); + + auto expect_hero_sizing = [&](const VTX::StructSchemaCache* s) { + ASSERT_NE(s, nullptr); + ASSERT_GT(s->array_max_indices.size(), stringIdx); + EXPECT_EQ(s->array_max_indices[floatIdx], 1); // Cooldowns + EXPECT_EQ(s->array_max_indices[stringIdx], 2); // Tags, Names + EXPECT_EQ(s->map_max_index, 1); // AmmoByWeapon + }; + + VTX::GenericNativeLoader native(cache); + VTX::GenericFlatBufferLoader flatbuffer(cache); + VTX::GenericProtobufLoader protobuf(schema); // proto loader takes the registry, not the cache + VTX::GenericBinaryLoader binary(cache); + + expect_hero_sizing(native.GetStructSizing(heroId)); + expect_hero_sizing(flatbuffer.GetStructSizing(heroId)); + expect_hero_sizing(protobuf.GetStructSizing(heroId)); + expect_hero_sizing(binary.GetStructSizing(heroId)); +} + TEST(NativeLoader, ADLConversionFromCustomMathType) { VTX::SchemaRegistry schema; ASSERT_TRUE(schema.LoadFromJson(SchemaPath())); diff --git a/tests/writer/test_roundtrip.cpp b/tests/writer/test_roundtrip.cpp index 8f5881d..7b8da63 100644 --- a/tests/writer/test_roundtrip.cpp +++ b/tests/writer/test_roundtrip.cpp @@ -12,6 +12,7 @@ #include "vtx/writer/core/vtx_writer_facade.h" #include "vtx/reader/core/vtx_reader_facade.h" #include "vtx/common/vtx_types.h" +#include "vtx/common/readers/schema_reader/schema_registry.h" #include "util/test_fixtures.h" @@ -378,6 +379,161 @@ TEST_P(RoundtripTest, RestoresBucketNamesFromSchemaOnRead) { EXPECT_EQ(frame_ref.GetBucket("entity").entities.size(), 1u); } +// --------------------------------------------------------------------------- +// Populated array and map field VALUES survive write -> read on both backends. +// (The existing scalar roundtrip does not cover FlatArray / map serialization.) +// Note: pre-sizing creates EMPTY declared slots in memory, but an array with no +// data is intentionally not serialized, so this covers the case that matters on +// disk -- populated arrays/maps. +// --------------------------------------------------------------------------- + +TEST_P(RoundtripTest, ArrayAndMapFieldValuesRoundtrip) { + auto cfg = MakeConfig("arraymap", "uuid-rt-arraymap"); + { + auto writer = CreateWriter(cfg); + ASSERT_TRUE(writer); + + VTX::Frame f; + auto& bucket = f.CreateBucket("entity"); + + VTX::PropertyContainer e; + e.entity_type_id = 0; + e.float_arrays.AppendSubArray({1.5f, 2.5f, 3.5f}); + e.string_arrays.AppendSubArray({std::string("alpha"), std::string("beta")}); + + VTX::MapContainer m; + m.keys = {"rifle", "pistol"}; + VTX::PropertyContainer v0; + v0.entity_type_id = 0; + v0.int32_properties = {30}; + VTX::PropertyContainer v1; + v1.entity_type_id = 0; + v1.int32_properties = {12}; + m.values.push_back(std::move(v0)); + m.values.push_back(std::move(v1)); + e.map_properties.push_back(std::move(m)); + + bucket.unique_ids.push_back("e0"); + bucket.entities.push_back(std::move(e)); + + VTX::GameTime::GameTimeRegister t; + t.game_time = 0.0f; + writer->RecordFrame(f, t); + writer->Stop(); + } + + auto ctx = VTX::OpenReplayFile(cfg.output_filepath); + ASSERT_TRUE(ctx) << ctx.error; + + const VTX::Frame* f = ctx.reader->GetFrameSync(0); + ASSERT_NE(f, nullptr); + ASSERT_EQ(f->GetBuckets().size(), 1u); + ASSERT_EQ(f->GetBuckets()[0].entities.size(), 1u); + const auto& e = f->GetBuckets()[0].entities[0]; + + // Float array survives with its values, in order. + ASSERT_EQ(e.float_arrays.SubArrayCount(), 1u); + const auto fa = e.float_arrays.GetSubArray(0); + ASSERT_EQ(fa.size(), 3u); + EXPECT_FLOAT_EQ(fa[0], 1.5f); + EXPECT_FLOAT_EQ(fa[1], 2.5f); + EXPECT_FLOAT_EQ(fa[2], 3.5f); + + // String array survives. + ASSERT_EQ(e.string_arrays.SubArrayCount(), 1u); + const auto sa = e.string_arrays.GetSubArray(0); + ASSERT_EQ(sa.size(), 2u); + EXPECT_EQ(sa[0], "alpha"); + EXPECT_EQ(sa[1], "beta"); + + // Map survives with keys and per-key values. + ASSERT_EQ(e.map_properties.size(), 1u); + ASSERT_EQ(e.map_properties[0].keys.size(), 2u); + EXPECT_EQ(e.map_properties[0].keys[0], "rifle"); + EXPECT_EQ(e.map_properties[0].keys[1], "pistol"); + ASSERT_EQ(e.map_properties[0].values.size(), 2u); + ASSERT_FALSE(e.map_properties[0].values[0].int32_properties.empty()); + EXPECT_EQ(e.map_properties[0].values[0].int32_properties[0], 30); + EXPECT_EQ(e.map_properties[0].values[1].int32_properties[0], 12); +} + +// --------------------------------------------------------------------------- +// Declared-but-empty array fields are restored on read. An array with no data +// is not serialized, so a reader must re-create the declared empty subarrays +// from the schema for the read frame to mirror an ingest-loaded frame's layout. +// (Maps already round-trip their slot count, so this covers only arrays.) +// --------------------------------------------------------------------------- + +namespace { + // Hero declares three array-typed fields (one float array, two string arrays, + // one struct array); an entity that leaves them empty exercises read-side + // restoration. + constexpr const char* kHeroArraySchema = R"({ + "version": "1.0.0", + "buckets": ["entity"], + "property_mapping": [ + { "struct": "Item", "values": [ + {"name":"Id","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}} + ]}, + { "struct": "Hero", "values": [ + {"name":"UniqueID","typeId":"String","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":1}}, + {"name":"Score","typeId":"Int32","containerType":"None","keyId":"None","structType":"","meta":{"defaultValue":"0","fixedArrayDim":1}}, + {"name":"Cooldowns","typeId":"Float","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Tags","typeId":"String","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Names","typeId":"String","containerType":"Array","keyId":"None","structType":"","meta":{"defaultValue":"","fixedArrayDim":0}}, + {"name":"Inventory","typeId":"Struct","containerType":"Array","keyId":"None","structType":"Item","meta":{"defaultValue":"","fixedArrayDim":0}} + ]} + ] + })"; +} // namespace + +TEST_P(RoundtripTest, ReaderRestoresDeclaredEmptyArrays) { + VTX::SchemaRegistry reg; + ASSERT_TRUE(reg.LoadFromRawString(kHeroArraySchema)); + const int32_t heroTypeId = reg.GetStructTypeId("Hero"); + ASSERT_GE(heroTypeId, 0); + + auto cfg = MakeConfig("declared_empty_arrays", "uuid-rt-declared-empty"); + cfg.schema_json_path.clear(); + cfg.schema_json_content = kHeroArraySchema; + { + auto writer = CreateWriter(cfg); + ASSERT_TRUE(writer); + + VTX::Frame f; + auto& bucket = f.CreateBucket("entity"); + VTX::PropertyContainer e; + e.entity_type_id = heroTypeId; + e.string_properties = {"hero_1"}; // only a scalar; all array fields left empty + bucket.unique_ids.push_back("hero_1"); + bucket.entities.push_back(std::move(e)); + + VTX::GameTime::GameTimeRegister t; + t.game_time = 0.0f; + writer->RecordFrame(f, t); + writer->Stop(); + } + + auto ctx = VTX::OpenReplayFile(cfg.output_filepath); + ASSERT_TRUE(ctx) << ctx.error; + + const VTX::Frame* f = ctx.reader->GetFrameSync(0); + ASSERT_NE(f, nullptr); + ASSERT_EQ(f->GetBuckets()[0].entities.size(), 1u); + const auto& e = f->GetBuckets()[0].entities[0]; + + // The declared array fields come back present-and-empty even though nothing + // was serialized for them (without read-side restoration these would be 0). + EXPECT_EQ(e.float_arrays.SubArrayCount(), 1u); // Cooldowns + EXPECT_EQ(e.string_arrays.SubArrayCount(), 2u); // Tags, Names + EXPECT_EQ(e.any_struct_arrays.SubArrayCount(), 1u); // Inventory + EXPECT_TRUE(e.float_arrays.GetSubArray(0).empty()); + EXPECT_TRUE(e.string_arrays.GetSubArray(0).empty()); + + // A type with no declared array field is untouched. + EXPECT_EQ(e.int32_arrays.SubArrayCount(), 0u); +} + // --------------------------------------------------------------------------- // Backend instantiation -- produces: // BothBackends/RoundtripTest.PreservesFrameData/FlatBuffers