fix: sector event side field and capturedFlag cleanup#143
Conversation
The old C++ extension passed sector event data raw 1:1 into JSON, which included the capturing side. The new typed pipeline was missing this field. - Add Side field to core.SectorEvent struct - Update parser to read side from arg[4], position from args[5-7] - Include side in v1 JSON export: [objectType, unitName, side, [pos]] - Include side in postgres message format - Update all tests with side field data and assertions
Summary of ChangesHello, 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 addresses a regression introduced in a previous refactor (#142) by restoring the Side field to core.SectorEvent struct. This field, which indicates the capturing or contesting side of a sector event, was inadvertently dropped. The changes ensure that this crucial information is now correctly parsed, stored, and exported, maintaining data integrity and consistency across the system. Highlights
Changelog
Activity
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.
Code Review
This pull request correctly adds the Side field to core.SectorEvent to preserve which side captured or contested a sector. The changes are consistently applied across parsing, storage backends (memory and postgres), and the v1 JSON export format. All relevant tests have been updated to cover the new field, including cases where the side is present, absent, or empty. The implementation is clean and follows the intentions described in the pull request. I have no further suggestions for improvement.
capturedFlag is no longer routed through :EVENT:SECTOR: in the addon. Remove it from parser comments, test cases, and builder format docs.
The v1 JSON spec wraps endMission data in an inner array: [frameNum, "endMission", [side, message]] Builder was incorrectly producing flat format: [frameNum, "endMission", side, message] This caused parser_v1.go to read only "side" as the message, losing the actual message text.
Merging this branch will not change overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
Summary
Sidefield inSectorEventthrough all storage backends (memory, postgres, websocket) and v1 JSON exportcapturedFlagreferences from sector event parser comments, test cases, and v1 builder format docs —capturedFlagis no longer routed through:EVENT:SECTOR:by the addonTest plan
TestParseSectorEventpasses (capturedFlag test case removed)TestBuildWithSectorEventspasses with side field in v1 JSON output