Visitar URL original
Inflict weapon shell bleeding, blackout and freeze on hit by erwan-joly · Pull Request #3183 · NosCoreIO/NosCore · GitHub
Skip to content

Inflict weapon shell bleeding, blackout and freeze on hit - #3183

Open
erwan-joly wants to merge 1 commit into
masterfrom
claude/project-thread-wi9f5y-shell-bleed
Open

Inflict weapon shell bleeding, blackout and freeze on hit#3183
erwan-joly wants to merge 1 commit into
masterfrom
claude/project-thread-wi9f5y-shell-bleed

Conversation

@erwan-joly

@erwan-joly erwan-joly commented Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Fixes #3163

What changed

Each weapon shell debuff option now becomes a Type95ChanceCausingXBuffOnEnemyOnAttack effect. The option's value is the chance, and the card is the matching debuff from the equipment family in Card.dat:

Option Card
Minor Bleeding 189
Bleeding 190
Heavy Bleeding 191
Blackout 196
Deadly Blackout 197
Freeze 200

Worn items such as weapon costumes use cards from the same family (196, 200) for their own on-attack debuffs. The on-hit service added in #3171 already rolls these effects for every landed hit, so the shell options need no new combat code.

The PR also rewords the race-damage step in the manual test plan, following the review on #3173. It now compares the range over several hits and uses a shell that carries only the race option.

Verified

Unit test: AWeaponShellsDebuffOptionsBecomeChancesToInflictTheirCard. The NosCore.GameObject.Tests suite passes.

In the client, a Swordsman transformed with the Warrior card hits a Baby Dander with a Short Sword r5. The shell carries Minor Bleeding at 100% so that every roll lands:

Over 10 hits
master 10 hits, no bf packet, no debuff over the Dander
this branch Every hit is followed by bf 3 990001 0.189.200 30. The Minor Bleeding icon shows over the Dander, and once the hits stop it loses 30 HP per tick (dm 3 990001 30).

master: Minor Bleeding 100% shell, the Dander takes the hit and shows no debuff

This branch: same hit, the Minor Bleeding icon shows over the Dander

Manual test plan

Added under Shells:

  • Hit a monster with a weapon shell carrying Minor Bleeding, Bleeding, Heavy Bleeding, Blackout, Deadly Blackout or Freeze. Now and then the debuff icon appears over the monster, and with bleeding its HP keeps dropping between hits.

Only Minor Bleeding was checked in the client. The other five options take the same path with a different card.

🤖 Generated with Claude Code

https://claude.ai/code/session_014gSmkVtYup7KTZ63CJcxZT

@coderabbitai

coderabbitai Bot commented Nov 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 27 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 9d10121 and fa066f7.

Files selected for processing: 3

  • documentation/manual-test-plan.md
  • src/NosCore.GameObject/Services/EquipmentService/EquipmentStatsService.cs
  • test/NosCore.GameObject.Tests/Services/EquipmentService/EquipmentStatsServiceTests.cs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@erwan-joly
erwan-joly force-pushed the claude/project-thread-wi9f5y-shell-bleed branch from a08732f to d0bbd97 Compare November 14, 2026 08:12
@claude
Inflict weapon shell bleeding, blackout and freeze on hit
fa066f7 
A weapon shell's Minor Bleeding, Bleeding, Heavy Bleeding, Blackout,
Deadly Blackout and Freeze options become chances to put the matching
equipment debuff card on the target of each landed hit, through the
same on-attack effect gear already uses.

Fixes #3163

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014gSmkVtYup7KTZ63CJcxZT
@erwan-joly
erwan-joly force-pushed the claude/project-thread-wi9f5y-shell-bleed branch from d0bbd97 to fa066f7 Compare November 14, 2026 08:18
@erwan-joly Claude

erwan-joly commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator Author

Review: the behaviour and the client check look right, but the option → card table is in the wrong place.

WeaponShellHitCard hardcodes cards 189-191, 196, 197 and 200 inside EquipmentStatsService. That is game data in server logic. The project rule is that such constants are written to the database by the parser (from the .dat files where possible, otherwise from a hardcoded table in NosCore.Parser), and the service reads them from there.

Suggested shape:

  • a parser table that maps each WeaponShellOptionType to its card (or a column on the shell option row if one fits);
  • EquipmentStatsService looks the card up from that data, like the other option mappings that come from the DB.

Holding the merge until that moves. Everything else (the BCard type, the chance taken from option.Value, reusing the on-hit roll from #3171) looks good.

Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Weapon shell bleeding, blackout and freeze options have no effect

2 participants

@erwan-joly @claude