Skip to content

Add forced movement guri packet fields - #513

Closed
NevileeV wants to merge 1 commit into
NosCoreIO:masterfrom
NevileeV:feature/forced-movement-guri
Closed

Add forced movement guri packet fields#513
NevileeV wants to merge 1 commit into
NosCoreIO:masterfrom
NevileeV:feature/forced-movement-guri

Conversation

@NevileeV

@NevileeV NevileeV commented Sep 1, 2026

Copy link
Copy Markdown

Extends the existing server GuriPacket with the three optional fields required by forced movement packets.

Adds serialization coverage for both push/pull and dash layouts while preserving existing shorter guri packets.

Verification:

  • dotnet build NosCore.Packets.sln
  • dotnet test NosCore.Packets.sln
  • 130 tests passed

Summary by CodeRabbit

  • New Features

    • Added support for three additional optional values in Guri packets.
    • Expanded packet serialization to include the new values.
    • Added support for serializing forced-movement Guri packets.
  • Tests

    • Added coverage for forced-movement packet serialization and updated expected serialized output.
  • Release

    • Updated the package version to 21.2.0.

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: e6791751-a387-4d12-b613-2ca89ede3acb

📥 Commits

Reviewing files that changed from the base of the PR and between 5218ba1 and 2790c86.

📒 Files selected for processing (4)
  • src/NosCore.Packets/NosCore.Packets.csproj
  • src/NosCore.Packets/ServerPackets/UI/GuriPacket.cs
  • test/NosCore.Packets.Tests/SerializerCorpusTests.ExpressionTreeSerializerOutputIsPinnedForEveryPacket.verified.txt
  • test/NosCore.Packets.Tests/SerializerTest.cs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change extends GuriPacket with three optional serialized values. Serializer tests and pinned corpus outputs include the new fields. The package version changes from 21.1.2 to 21.2.0.

Changes

Guri packet serialization

Layer / File(s) Summary
Extend Guri packet serialization
src/NosCore.Packets/ServerPackets/UI/GuriPacket.cs, test/NosCore.Packets.Tests/SerializerTest.cs, test/NosCore.Packets.Tests/SerializerCorpusTests.ExpressionTreeSerializerOutputIsPinnedForEveryPacket.verified.txt, src/NosCore.Packets/NosCore.Packets.csproj
GuriPacket adds nullable ThirdValue, FourthValue, and FifthValue properties at indexes 6–8. Parameterized tests and pinned serializer outputs include the new values. The package version is 21.2.0.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2790c

The change adds optional forced-movement values while preserving shorter Guri packets and includes passing build and test verification. No actionable merge-blocking risk remains beyond normal checks and review.

Suggested reviewers: erwan-joly, denislauri1999

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding forced movement fields to the GuriPacket.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@NevileeV

NevileeV commented Sep 1, 2026

Copy link
Copy Markdown
Author

All checks are green and the full local test suite passes (130/130). This package change is required by the general Type 40 forced-movement refactor in NosCore. Could a maintainer please review and merge it?

@NevileeV

NevileeV commented Sep 1, 2026

Copy link
Copy Markdown
Author

Closing because this change will be used only in a local private build.

@NevileeV NevileeV closed this Sep 1, 2026
@NevileeV
NevileeV deleted the feature/forced-movement-guri branch September 1, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant