Skip to content

Group friendly fire: block at the source seams (3/4, part of #277) - #303

Open
Zaldaryon wants to merge 1 commit into
feat/issue-277-ff-explosionsfrom
feat/issue-277-ff-source-seams
Open

Group friendly fire: block at the source seams (3/4, part of #277)#303
Zaldaryon wants to merge 1 commit into
feat/issue-277-ff-explosionsfrom
feat/issue-277-ff-source-seams

Conversation

@Zaldaryon

Copy link
Copy Markdown
Contributor

Summary

Third of four stacked follow-ups to #300. Moves the group friendly fire block from the health check up to the two seams where an attack is actually decided, which fixes three of #300's stated limitations at once.

Base branch is feat/issue-277-ff-explosions (PR 2/4).

Why

The #300 hook in EntityPlayer.ShouldReceiveDamage stops the damage but runs late. By the time it fires, a melee swing has already:

  • played sounds/player/slap (EntityAgent.OnInteract),
  • spent a point of weapon durability (Collectible.OnAttackingWith calls DamageItem),
  • fired OnPlayerInteractEntity, which combat-tags both players (StratumCombatLogSystem).

A blocked shot similarly still paid the arrow its durability and impact sound in ImpactOnEntity.

Change

  • sources/VintagestoryApi/Server/StratumFriendlyFireHook.cs: adds OnBlockedAttack, an Action<string,string> the seams call so the notice logic lives in one place.
  • sources/VintagestoryLib/Vintagestory.Server/StratumFriendlyFireGuard.cs (new): BlocksMeleeAttack, so the vanilla patch stays a one-line call.
  • patches/VintagestoryLib/Vintagestory.Server/ServerSystemEntitySimulation.cs.patch: the guard call in HandleEntityInteraction, right after the vanilla PvP gate, only for attacks (flag), never right-click interactions like healing a group mate. Returning there is exactly the vanilla PvP-off path: no interact event, so no slap, no durability, no combat tag.
  • patches/VSEssentials/Entities/EntityProjectileBase.cs.patch: the same rule inlined in CanDealDamage next to the existing AllowPvP check (VSEssentials cannot reference VintagestoryLib, so it uses the API-only types and fires the hook directly).
  • sources/VintagestoryLib/Vintagestory.Server/StratumFriendlyFireSystem.cs: OnBlockedAttack points at a per-attacker throttled chat notice and a blocked-hit counter shown in /friendlyfire status.
  • sources/VintagestoryLib/Vintagestory.Server/StratumFriendlyFireConfig.cs: NotifyBlockedAttacker (default true), NotifyThrottleMs (default 3000, clamped 500 to 60000), BlockedMessage. StratumConfig wires the EnsureSane call.
  • docs/commands/friendlyfire.md: the new config keys and the status counter.

The ShouldReceiveDamage hook from #300 stays as the catch-all for mod-authored damage that calls ReceiveDamage normally, and as the test source for the explosion filter.

No combat tag on a blocked hit

This inverts what #300's body asked for. Today a blocked melee swing wrongly tags both players (the projectile path already does not tag, gated on didDamage). A hit that deals no damage is not combat, and letting a group mate tag you by holding left-click is an exploit. The melee seam returning before TriggerPlayerInteractEntity fixes it for free, no change to StratumCombatLogSystem.

Gates

  • Two-pass build green, 4 pre-existing NU1904.
  • bash scripts/smoke-test.sh: PASS ... 13 console command(s) verified.
  • scripts/extract-patches.sh: ServerSystemEntitySimulation.cs.patch and EntityProjectileBase.cs.patch only. The SSE patch has one two-line brace reshuffle from the local decompiler in an unrelated adjacent hunk, otherwise the diff is the new melee hunk plus offset bumps. ServerMain.cs.patch from PR 2 is left untouched.
  • Private Atlas suite: the control-path scenarios pass. The melee/projectile durability and no-tag scenarios need two connected players and are blocked by the same 1.22.7 synthetic-join gap as Stack multiple role prefixes on one player, ordered by Priority #274's suite.

Limitations

  • With friendly fire off, swinging at a group mate no longer plays the slap sound. Intended.
  • A mod that Harmony-replaces Entity.ReceiveDamage, the interaction handler, or the projectile path and skips the original still bypasses this. PR 4/4 logs a warning naming such a mod. No server platform can hard-prevent it.
  • Blocked hits stay out of the combat log entirely (deliberate); the /friendlyfire status counter is the visibility.

Type

  • Bug fix
  • Performance
  • New feature
  • Refactor or cleanup
  • Docs or build

Checklist

  • scripts/extract-patches.sh ran clean.
  • dotnet build VintageStory.slnx -c Release -p:EmbedPatchedFiles=true is green.
  • Every vanilla edit has a // Stratum marker.
  • No vanilla source committed.
  • Tested on a real server start, not just compilation.

Related issues

Part of #277

The #300 hook in EntityPlayer.ShouldReceiveDamage stops the damage but
runs late: for melee it is already past the slap sound, the weapon
durability tick in Collectible.OnAttackingWith, and the interact event
that combat-tags both players. A blocked shot likewise still paid the
arrow its durability and impact sound.

This adds the group check at the two seams where an attack is decided:

- HandleEntityInteraction, right after the vanilla PvP gate, through
  StratumFriendlyFireGuard. Returning there is exactly the vanilla
  PvP-off path: no interact event, so no slap, no durability loss, no
  combat-log tag.
- EntityProjectileBase.CanDealDamage, beside the existing AllowPvP
  check. ImpactOnEntity then skips the sound, the damage, and the
  projectile durability hit.

Both fire StratumFriendlyFireHook.OnBlockedAttack, which
StratumFriendlyFireSystem points at a per-attacker throttled notice
(FriendlyFire.NotifyBlockedAttacker / NotifyThrottleMs / BlockedMessage)
and a blocked-hit counter shown in /friendlyfire status. The
ShouldReceiveDamage hook stays as the catch-all for mod-authored damage
and for the explosion test source.

No combat tag on a blocked hit is deliberate: a hit that deals no damage
is not combat, and letting a group mate tag you by spam-clicking would
be an exploit.

Part of #277
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