Buccaneer Upgrade / Barnacle #11
Conversation
…Fix incorrect memory use calculation
New barnacle based branch of buccaneer, with code changes from Gemini Code assist
- Added new logic to write to file without opening/closing it
- Fix TEXT arrays issue
- Final fix
…onModule.cpp Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Added protection from writing into a corrupted file that is not big enough
Update file rewrite handling
Better describes the function
… functionality. Tested and working. Co-authored-by: Luke Bermingham <[email protected]> Denis was here too!
Introduces FMetricsCollection and FBuccaneerMetric structures for unified metrics representation. Replaces ad-hoc JSON construction with structured metrics collection and serialization in BuccaneerCommon, BuccaneerStats, and PixelStreaming modules. Updates interfaces and implementations to use SendMetrics instead of SendStats, improving code clarity and extensibility for metrics reporting. Co-Authored-By: Denis Phoenix <[email protected]> Co-Authored-By: Luke Bermingham <[email protected]>
Good commit for Denis and Michael to practice/attempt reformat of JSON structure.
… correctly. Tested with PS1/2
Fixing compile error with consts
… compose up work for local dev, improved stat name sanitizing
…aneer into barnacle-gemini
…terval cvar for Bucc4PS1 plugin
Summary of ChangesHello @MWillWallT, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the Buccaneer metrics and event pipeline, integrating it more deeply and robustly across Unreal Engine plugins and the Go server. The primary goal is to provide a unified, maintainable architecture for metrics, especially enabling first-class support for Pixel Streaming v1 and v2. It also focuses on enhancing the reliability of the Buccaneer server and streamlining the local development experience through updated Docker configurations and comprehensive CVar options. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive refactoring of the Buccaneer metrics and events pipeline, introducing support for Pixel Streaming v1 and v2 with improved modularity, thread safety, and configuration management. The refactor splits the monolithic BuccaneerCommon module into dedicated BuccaneerStats, BuccaneerEvents, and BuccaneerMetrics components, while adding new Pixel Streaming integration plugins and enhancing the Go server with better concurrency handling and configurability.
Key Changes:
- Refactored metrics architecture using
FMetricsCollectionfor unified stats handling across Pixel Streaming v1, v2, and core application metrics - Added thread-safe mutex locking in the Go server to prevent race conditions and deadlocks
- Introduced new
Buccaneer4PixelStreaming2plugin alongside refactoredBuccaneer4PixelStreamingplugin - Split Docker Compose configurations into development-only (
docker-compose.yml) and full demo (docker-compose-all.yml) setups
Reviewed Changes
Copilot reviewed 56 out of 59 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
Server/BuccaneerServer/main.go |
Added configurable port via CLI, mutex locking for thread safety, graceful handling of missing descriptions, and health check endpoint |
Server/BuccaneerServer/Dockerfile |
Simplified COPY commands for go.mod and go.sum |
Plugins/Buccaneer4PixelStreaming2/** |
New plugin for Pixel Streaming v2 with module interfaces, settings, logging, and stats collection |
Plugins/Buccaneer4PixelStreaming/** |
Refactored v1 plugin with new module interfaces, settings system, and improved metrics handling |
Plugins/Buccaneer/Source/BuccaneerCommon/** |
Refactored into modular components with IBuccaneerCommonModule, BuccaneerSettings, and BuccaneerMetrics |
Plugins/Buccaneer/Source/BuccaneerStats/** |
New dedicated stats module replacing TimeSeriesDataEmitter |
Plugins/Buccaneer/Source/BuccaneerEvents/** |
New dedicated events module replacing SemanticEventEmitter |
readme.md |
Updated documentation distinguishing dev setup from full demo |
Examples/Compose/docker-compose.yml |
Modified for development workflow (monitoring stack only) |
Examples/Compose/docker-compose-all.yml |
New file for complete demo including Unreal application |
Plugins/Buccaneer/Buccaneer.uplugin |
Updated module names from old to new naming scheme |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a significant and well-executed pull request that refactors the metrics pipeline, introduces new integrations, and hardens the Buccaneer server. The separation of concerns into distinct modules like BuccaneerCommon, BuccaneerMetrics, and BuccaneerSettings is a major improvement for maintainability. The fixes for race conditions and deadlocks in the Go server are crucial for stability. I've identified a few issues, including a critical bug in metadata parsing, a couple of high-severity correctness and concurrency bugs, and some medium-severity items related to performance and code clarity. Once these are addressed, this will be an excellent upgrade.
Plugins/Buccaneer/Source/BuccaneerCommon/Private/BuccaneerSettings.cpp
Outdated
Show resolved
Hide resolved
Plugins/Buccaneer/Source/BuccaneerCommon/Private/BuccaneerCommonModule.cpp
Show resolved
Hide resolved
Plugins/Buccaneer/Source/BuccaneerStats/Private/BuccaneerStatsModule.cpp
Outdated
Show resolved
Hide resolved
Plugins/Buccaneer/Source/BuccaneerCommon/Private/BuccaneerCommonModule.cpp
Outdated
Show resolved
Hide resolved
Plugins/Buccaneer/Source/BuccaneerCommon/Private/BuccaneerCommonModule.cpp
Show resolved
Hide resolved
|
/gemini review |
There was a problem hiding this comment.
Code Review
This is a substantial pull request that refactors the metrics pipeline, introduces new integrations for Pixel Streaming, and improves the Buccaneer server. The changes are well-structured and significantly improve the modularity and configurability of the plugins. The move to UDeveloperSettings and dedicated modules for stats and events is a great improvement.
I've found a few issues, mainly related to a race condition in the Go server and a copy-paste error in the new Grafana dashboards. I've also noted a potential improvement for the Docker Compose setup. My detailed comments are below.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
| // Case: Sending stats to disk | ||
| if (UBuccaneerSettings::CVarEnableJSONOutput.GetValueOnAnyThread()) | ||
| { | ||
| WriteJSON(TEXT("stats.json"), JsonObject); |
There was a problem hiding this comment.
Maybe we should dump the stats with a unique file name that is based on a combination of BucID and time.
So something then ends up being in the format {BucId}_{UnixTimestamp}_Stats.json (this is not valid syntax btw, just an example of the file name pattern).
There was a problem hiding this comment.
Aha coincidental you bring this up. I'm working on an arg that allows you to specify the filename if desired. We should chat on this
There was a problem hiding this comment.
Updated alongside the new filename arg. See commit here: b3013f2
lukehb
left a comment
lukehb
left a comment
There was a problem hiding this comment.
Minor change requested about produced json file.
New arg: -BuccaneerJSONOutputFile=<filename> Tested to ensure it works with or without .json file extension. Default output filename has been updated to BuccID_Unixtime. Unix time is based on application start time. Using -BuccaneerID= allows users to specify the BuccID in the default name output. Tested changes / args to ensure robustness.
Summary
This PR merges the
barnacle-geminiwork intomaster, introducing a refactored Buccaneer metrics pipeline, new Pixel Streaming integrations (v1 & v2), and multiple robustness and observability improvements across the Unreal plugins and Buccaneer server.Key Changes
Metrics refactor and configuration
FMetricsCollection, reducing duplication and simplifying stat management.BuccaneerCommoninto clearer module boundaries withBuccaneerCommonModule,BuccaneerMetrics, andBuccaneerSettingssplit into dedicated source files.</>formatting for metric names.ResX/ResY) and updated metric sending logic while preserving backwards‑compatible behaviour with existing Buccaneer functionality.IBuccaneerCommonModule).Pixel Streaming integrations
Buccaneer4PixelStreamingwith cleaner separation between public and private headers.Buccaneer4PixelStreaming2plugin for Pixel Streaming v2, including its own module interfaces, settings, and logging.Events & stats modules
BuccaneerEventsandBuccaneerStatsas first‑class modules with their own build files, module interfaces, logging, and blueprint function libraries.Logging.cpp/Logging.hper module) for more consistent diagnostics.Buccaneer server improvements
BuccaneerServer(Go) to improve robustness and avoid deadlocks, including:main.gofor better server observability and debugging.Docker / local dev workflow
Examples/Compose/docker-compose.ymland addeddocker-compose-all.ymlto support running the full stack for local development.Server/BuccaneerServer/Dockerfilesodocker-compose upworks more smoothly for local dev and testing.Configuration & documentation
readme.mdto align with the new Barnacle/Buccaneer integration and Pixel Streaming workflows..uplugindescriptors for the Buccaneer and Pixel Streaming plugins, including the newBuccaneer4PixelStreaming2plugin and icon resources.Dashboards & stability fixes
Motivation / Rationale
Testing
FMetricsCollectionrefactor.docker-compose.ymlanddocker-compose-all.yml) start the expected services for local development.New CVars and command-line flags (including JSON output)
This PR also adds/standardises a set of CVars and command-line flags to make it easy to point Buccaneer at a server, toggle stats/events, and optionally write JSON locally for inspection.
Core Buccaneer CVars / flags
Buccaneer.URL→-BuccaneerURL="http://127.0.0.1:8000"Buccaneer.ID→-BuccaneerID="MyMachine01"Buccaneer.EnableStats→-BuccaneerEnableStats=true|falseBuccaneer.EnableEvents→-BuccaneerEnableEvents=true|falseBuccaneer.Metadata→-BuccaneerMetadata=Key1:Value1;Key2:Value2Buccaneer.ReportingInterval→-BuccaneerReportingInterval=1.0Buccaneer.EnableJSONOutput→-BuccaneerEnableJSONOutput=true|falseBuccaneer.JSONOutputDirectory→-BuccaneerJSONOutputDirectory="C:/Temp/Buccaneer"BuccaneerEnableJSONOutputis enabled.Example end-to-end launch line (local Barnacle dev):
Pixel Streaming–specific CVars / flags
Pixel Streaming 1 (
Buccaneer4PixelStreaming):Buccaneer4PixelStreaming.EnableStats→-Buccaneer4PixelStreamingEnableStats=true|falseBuccaneer4PixelStreaming.ReportingInterval→-Buccaneer4PixelStreamingReportingInterval=0.5<= 0disables reporting.Pixel Streaming 2 (
Buccaneer4PixelStreaming2):Buccaneer4PixelStreaming2.EnableStats→-Buccaneer4PixelStreaming2EnableStats=true|falseBuccaneer4PixelStreaming2.ReportingInterval→-Buccaneer4PixelStreaming2ReportingInterval=0.5Example Pixel Streaming 2 launch line:
Breaking Changes / Migration Notes
BuccaneerCommonsplit into module, metrics, and settings types; some headers moved fromPublictoPrivate). Projects using internal or previously non‑intended public headers may need to update includes to the new module interfaces.