Skip to content

Clanker APR Nerf (Crafted droids)#1956

Open
LazyTrain082 wants to merge 9 commits intozunath:masterfrom
LazyTrain082:crafted-droid-nerfs
Open

Clanker APR Nerf (Crafted droids)#1956
LazyTrain082 wants to merge 9 commits intozunath:masterfrom
LazyTrain082:crafted-droid-nerfs

Conversation

@LazyTrain082
Copy link
Copy Markdown
Contributor

@LazyTrain082 LazyTrain082 commented Mar 27, 2026

Wave 1 of a possible 3 wave Droid nerf.

This specifically fixes two things:

Stat.cs:
ApplyAttacksPerRound() now detects droids and reads perk levels from Droid.GetControllerItem(creature) + Droid.LoadDroidItemPropertyDetails(controller).Perks

APR checks all relevant perks.

Previously, Stat.ApplyAttacksPerRound() was using Perk.GetPerkLevel() for droids. For NPCs, Perk.GetPerkLevel() falls back to max perk level if local PERK_LEVEL_* isn’t set, which meant crafted droids were getting an extra 5 APR just for showing up.

And just for good measure I reconfigured the free Mastery perks they were getting, meaning they should now perform at-level with a T5 PC.

If they continue to need nerfs, I will drop their max stats by 5 for T5, T4, and T3, meaning they'll be locked to T4.5 equivalent PC.

If they continue to need nerfs after that I'll take away Mastery 2 from T5 and set it to Mastery 1.

Summary by CodeRabbit

  • New Features

    • Droids now automatically recalc attacks-per-round when equipping or unequipping items and use perk levels defined on their controller item.
  • Bug Fixes

    • Fixed droid attack-speed calculations to correctly use droid-specific perk levels.
  • Balance Changes

    • Moved several droid weapon mastery perks from Tier 3 to Tier 4.

Wave 1 of a possible 3 wave Droid nerf.

This specifically fixes two things:

Stat.cs:
ApplyAttacksPerRound() now detects droids and reads perk levels from Droid.GetControllerItem(creature) + Droid.LoadDroidItemPropertyDetails(controller).Perks

APR checks all relevant perks.

Previously, Stat.ApplyAttacksPerRound() was using Perk.GetPerkLevel() for droids. For NPCs, Perk.GetPerkLevel() falls back to max perk level if local PERK_LEVEL_* isn’t set, which meant crafted droids were getting an extra 5 APR just for showing up.

And just for good measure I reconfigured the free Mastery perks they were getting, meaning they should now perform at-level with a T5 PC.

If they continue to need nerfs, I will drop their max stats by 5 for T5, T4, and T3, meaning they'll be locked to T4.5 equivalent PC.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 27, 2026

Warning

Rate limit exceeded

@zunath has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 45 minutes and 13 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: d69acc3d-fc25-4d23-90ec-c010a8379f2c

📥 Commits

Reviewing files that changed from the base of the PR and between e22f05c and e926ad1.

📒 Files selected for processing (3)
  • SWLOR.Game.Server/Feature/EquipmentRestrictions.cs
  • SWLOR.Game.Server/Service/Droid.cs
  • SWLOR.Game.Server/Service/Stat.cs
📝 Walkthrough

Walkthrough

Moves ten weapon mastery perks from Tier 3 to Tier 4 in droid defaults, adds a droid-specific perk-level resolver, exposes Droid.ApplyDroidAttacksPerRound(...), and invokes it from equipment equip/unequip flows to recompute droid attacks-per-round using controller-stored perk levels.

Changes

Cohort / File(s) Summary
Droid service
SWLOR.Game.Server/Service/Droid.cs
Reassigned 10 weapon mastery perks from Tier 3 → Tier 4 in CacheDefaultTierPerks(). Added BuildPerkLevelResolver(uint droid) and new public ApplyDroidAttacksPerRound(uint droid, uint rightHandWeapon, uint offHandItem = OBJECT_INVALID) that routes droids to Stat.ApplyAttacksPerRound with a controller-perk resolver.
Stat calculations
SWLOR.Game.Server/Service/Stat.cs
ApplyAttacksPerRound signature accepts optional Func<PerkType,int> perkLevelResolver. Consolidated local perk lookups into GetAPRPerkLevel(PerkType) which uses the resolver when provided (fallback to Perk.GetPerkLevel). Added using System.Collections.Generic;.
Equipment restrictions / triggers
SWLOR.Game.Server/Feature/EquipmentRestrictions.cs
After equip/unequip trigger processing, detect droids and recompute current right/left hand items (accounting for the equip/unequip slot), then call Droid.ApplyDroidAttacksPerRound(player, rightHand, leftHand) to apply updated attacks-per-round.

Sequence Diagram(s)

sequenceDiagram
    participant Player
    participant EquipFeature as EquipmentRestrictions
    participant DroidSvc as Droid.Service
    participant StatSvc as Stat.Service

    Player ->> EquipFeature: Equip/Unequip item
    EquipFeature ->> EquipFeature: Run equip/unequip triggers
    EquipFeature ->> DroidSvc: Compute right/left hand items (with slot override)
    EquipFeature ->> DroidSvc: ApplyDroidAttacksPerRound(player, rightHand, leftHand)
    DroidSvc ->> DroidSvc: LoadDroidItemPropertyDetails(controller) -> build resolver
    DroidSvc ->> StatSvc: ApplyAttacksPerRound(droid, rightHand, leftHand, resolver)
    StatSvc ->> StatSvc: Resolve perk levels via resolver or Perk.GetPerkLevel
    StatSvc -->> DroidSvc: Updated attacks-per-round applied
    DroidSvc -->> EquipFeature: done
    EquipFeature -->> Player: finish equip/unequip
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nudged ten perks from three to four,

I hopped the resolver to check the core.
When gears equip and circuits hum,
I tap the stat and updates come.
— a little rabbit, proud and numb 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'Clanker APR Nerf (Crafted droids)' directly reflects the main objective: reducing attack-per-round values for crafted droids by fixing perk level detection logic.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
SWLOR.Game.Server/Service/Stat.cs (1)

1645-1654: Minor: Droid without valid controller gets zero perks.

If GetControllerItem returns an invalid object, droidPerks remains an empty dictionary, causing all perk lookups to return 0. This is likely intentional (a droid without a controller shouldn't get bonuses), but worth confirming this edge case is expected behavior.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@SWLOR.Game.Server/Service/Stat.cs` around lines 1645 - 1654, The code
initializes droidPerks to an empty Dictionary which makes lookups return zero
when Droid.GetControllerItem(controller) is invalid; change the logic in the
Droid handling block so droidPerks remains null unless a valid controller is
found (i.e., remove the early new Dictionary<PerkType,int>() assignment and only
assign droidPerks = Droid.LoadDroidItemPropertyDetails(controller).Perks when
GetIsObjectValid(controller) is true) so downstream code can distinguish "no
controller" (null) from "controller with zero perks" when using Droid.IsDroid,
Droid.GetControllerItem, and Droid.LoadDroidItemPropertyDetails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@SWLOR.Game.Server/Service/Stat.cs`:
- Around line 1645-1654: The code initializes droidPerks to an empty Dictionary
which makes lookups return zero when Droid.GetControllerItem(controller) is
invalid; change the logic in the Droid handling block so droidPerks remains null
unless a valid controller is found (i.e., remove the early new
Dictionary<PerkType,int>() assignment and only assign droidPerks =
Droid.LoadDroidItemPropertyDetails(controller).Perks when
GetIsObjectValid(controller) is true) so downstream code can distinguish "no
controller" (null) from "controller with zero perks" when using Droid.IsDroid,
Droid.GetControllerItem, and Droid.LoadDroidItemPropertyDetails.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc54819e-0495-4e5e-b824-a8952d9c972e

📥 Commits

Reviewing files that changed from the base of the PR and between 0b127da and c1891db.

📒 Files selected for processing (2)
  • SWLOR.Game.Server/Service/Droid.cs
  • SWLOR.Game.Server/Service/Stat.cs

Comment thread SWLOR.Game.Server/Service/Stat.cs
Comment thread SWLOR.Game.Server/Service/Stat.cs Outdated
Comment thread SWLOR.Game.Server/Service/Stat.cs Outdated
@LazyTrain082
Copy link
Copy Markdown
Contributor Author

@coderabbitai re-review with your latest changes implemented, ScriptSquirrel.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 4, 2026

@LazyTrain082 Sure! Kicking off a fresh review of PR #1956 with the latest changes now. 🐇


🧠 Learnings used
Learnt from: zunath
Repo: zunath/SWLOR_NWN PR: 1960
File: SWLOR.Game.Server/Feature/AbilityDefinition/Armor/ProvokeAbilityDefinition.cs:30-33
Timestamp: 2026-03-30T00:21:48.073Z
Learning: In SWLOR_NWN (C#), all droids (identified by ResRef via `Droid.IsDroid()`) are treated as player associates by design. Ownership verification is not needed when checking for droids in ability logic — if a creature is a droid (any summoned droid), it should be excluded from player-targeting abilities like Provoke. Do not suggest adding ownership-aware predicates for droid checks.
✅ Actions performed

Full review triggered.

@LazyTrain082
Copy link
Copy Markdown
Contributor Author

@coderabbitai re-review now that we're not rate-limited with your latest suggestions implemented

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 4, 2026

@LazyTrain082 Sure! Triggering a fresh full review of PR #1956 with your latest changes now. 🐇


🧠 Learnings used
Learnt from: zunath
Repo: zunath/SWLOR_NWN PR: 1960
File: SWLOR.Game.Server/Feature/AbilityDefinition/Armor/ProvokeAbilityDefinition.cs:30-33
Timestamp: 2026-03-30T00:21:48.073Z
Learning: In SWLOR_NWN (C#), all droids (identified by ResRef via `Droid.IsDroid()`) are treated as player associates by design. Ownership verification is not needed when checking for droids in ability logic — if a creature is a droid (any summoned droid), it should be excluded from player-targeting abilities like Provoke. Do not suggest adding ownership-aware predicates for droid checks.
✅ Actions performed

Full review triggered.

@LazyTrain082
Copy link
Copy Markdown
Contributor Author

@LazyTrain082 Sure! Triggering a fresh full review of PR #1956 with your latest changes now. 🐇
🧠 Learnings used

✅ Actions performed

image

CodeRabbit I need the review..........

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
SWLOR.Game.Server/Feature/EquipmentRestrictions.cs (1)

277-288: Extract duplicated hand-state rebuild logic into one helper.

The equip and unequip paths duplicate the same right/left-hand recompute flow; consolidating this will reduce drift risk.

♻️ Suggested refactor
@@
-            if (Droid.IsDroid(player))
-            {
-                var rightHand = GetItemInSlot(InventorySlot.RightHand, player);
-                var leftHand = GetItemInSlot(InventorySlot.LeftHand, player);
-
-                if (slot == InventorySlot.RightHand)
-                    rightHand = item;
-                else if (slot == InventorySlot.LeftHand)
-                    leftHand = item;
-
-                Droid.ApplyDroidAttacksPerRound(player, rightHand, leftHand);
-            }
+            if (Droid.IsDroid(player))
+            {
+                ApplyDroidAttacksPerRoundForPendingHandChange(player, slot, item, isUnequip: false);
+            }
@@
-            if (Droid.IsDroid(player))
-            {
-                var slot = Item.GetItemSlot(player, item);
-                var rightHand = GetItemInSlot(InventorySlot.RightHand, player);
-                var leftHand = GetItemInSlot(InventorySlot.LeftHand, player);
-
-                if (slot == InventorySlot.RightHand)
-                    rightHand = OBJECT_INVALID;
-                else if (slot == InventorySlot.LeftHand)
-                    leftHand = OBJECT_INVALID;
-
-                Droid.ApplyDroidAttacksPerRound(player, rightHand, leftHand);
-            }
+            if (Droid.IsDroid(player))
+            {
+                var slot = Item.GetItemSlot(player, item);
+                ApplyDroidAttacksPerRoundForPendingHandChange(player, slot, item, isUnequip: true);
+            }
+
+        private static void ApplyDroidAttacksPerRoundForPendingHandChange(uint player, InventorySlot slot, uint item, bool isUnequip)
+        {
+            var rightHand = GetItemInSlot(InventorySlot.RightHand, player);
+            var leftHand = GetItemInSlot(InventorySlot.LeftHand, player);
+
+            if (slot == InventorySlot.RightHand)
+                rightHand = isUnequip ? OBJECT_INVALID : item;
+            else if (slot == InventorySlot.LeftHand)
+                leftHand = isUnequip ? OBJECT_INVALID : item;
+
+            Droid.ApplyDroidAttacksPerRound(player, rightHand, leftHand);
+        }

Also applies to: 319-331

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@SWLOR.Game.Server/Feature/EquipmentRestrictions.cs` around lines 277 - 288,
Extract the duplicated right/left hand rebuild logic into a single helper (e.g.,
UpdateDroidHandState or RecomputeDroidHands) that accepts the player, the slot
being changed and the new item; inside it call
GetItemInSlot(InventorySlot.RightHand/LeftHand, player), assign the passed item
to the appropriate local (based on the slot) and then call
Droid.ApplyDroidAttacksPerRound(player, rightHand, leftHand) if
Droid.IsDroid(player). Replace the duplicated blocks in both the equip and
unequip paths with a call to this new helper to ensure a single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@SWLOR.Game.Server/Feature/EquipmentRestrictions.cs`:
- Around line 277-288: Extract the duplicated right/left hand rebuild logic into
a single helper (e.g., UpdateDroidHandState or RecomputeDroidHands) that accepts
the player, the slot being changed and the new item; inside it call
GetItemInSlot(InventorySlot.RightHand/LeftHand, player), assign the passed item
to the appropriate local (based on the slot) and then call
Droid.ApplyDroidAttacksPerRound(player, rightHand, leftHand) if
Droid.IsDroid(player). Replace the duplicated blocks in both the equip and
unequip paths with a call to this new helper to ensure a single source of truth.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d4027107-f2f5-4078-a48e-12e228e21258

📥 Commits

Reviewing files that changed from the base of the PR and between 350e2af and 9229ef7.

📒 Files selected for processing (3)
  • SWLOR.Game.Server/Feature/EquipmentRestrictions.cs
  • SWLOR.Game.Server/Service/Droid.cs
  • SWLOR.Game.Server/Service/Stat.cs
🚧 Files skipped from review as they are similar to previous changes (1)
  • SWLOR.Game.Server/Service/Droid.cs

@LazyTrain082
Copy link
Copy Markdown
Contributor Author

@zunath Good to go

@LazyTrain082
Copy link
Copy Markdown
Contributor Author

@coderabbitai Confirm all conflicts and requests are resolved. Is this ready to merge?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 4, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get overall PR status
gh pr view 1956 --repo zunath/SWLOR_NWN --json title,state,mergeable,mergeStateStatus,reviews,reviewRequests,statusCheckRollup,comments

echo "---REVIEW THREADS---"
# Get review threads to check for unresolved comments
gh api graphql -f query='
{
  repository(owner: "zunath", name: "SWLOR_NWN") {
    pullRequest(number: 1956) {
      reviewThreads(first: 50) {
        nodes {
          isResolved
          comments(first: 1) {
            nodes {
              author { login }
              body
            }
          }
        }
      }
    }
  }
}'

Length of output: 31632


@LazyTrain082 Here's the current status of PR #1956:

No merge conflicts — the branch is CLEAN and MERGEABLE.
No pending review requests.
CI/status checks passing.

Not ready to merge — there are 3 unresolved review threads from @zunath (the repo owner) that need to be addressed:

  1. "Why are DM and DM-possessed creatures not included in this now?" — A question about the DM/DM-possessed early-return logic in ApplyAttacksPerRound.
  2. "@coderabbitai - Did you review this code?" — A request directed at me to review a specific section.
  3. "@coderabbitai - This all feels like a code smell. Droid-specific functionality is now in a shared service and it probably should be in more droid-specific locations. Please review." — A design concern about droid logic living in the shared Stat.cs service rather than a droid-specific location.

Additionally, my latest review left 2 open nitpick comments (non-blocking, but worth noting):

  • EquipmentRestrictions.cs: The duplicated right/left-hand rebuild logic in the equip and unequip paths could be extracted into a shared helper to reduce drift risk.
  • Stat.cs: A droid with no valid controller silently gets zero perks — this was flagged as worth confirming whether the behavior is intentional.

The owner's three threads are the blockers. I'd recommend addressing @zunath's concerns — particularly the design question about keeping droid-specific logic out of the shared Stat.cs service — before this is merged. Would you like me to do a deeper review on the code smell concern?


🧠 Learnings used
Learnt from: zunath
Repo: zunath/SWLOR_NWN PR: 1960
File: SWLOR.Game.Server/Feature/AbilityDefinition/Armor/ProvokeAbilityDefinition.cs:30-33
Timestamp: 2026-03-30T00:21:48.073Z
Learning: In SWLOR_NWN (C#), all droids (identified by ResRef via `Droid.IsDroid()`) are treated as player associates by design. Ownership verification is not needed when checking for droids in ability logic — if a creature is a droid (any summoned droid), it should be excluded from player-targeting abilities like Provoke. Do not suggest adding ownership-aware predicates for droid checks.

@LazyTrain082
Copy link
Copy Markdown
Contributor Author

Tip

For best results, initiate chat on the files or code changes.
🧩 Analysis chain

@LazyTrain082 Here's the current status of PR #1956:

No merge conflicts — the branch is CLEAN and MERGEABLE. ✅ No pending review requests.CI/status checks passing.

Not ready to merge — there are 3 unresolved review threads from @zunath (the repo owner) that need to be addressed:

1. **"Why are DM and DM-possessed creatures not included in this now?"** — A question about the DM/DM-possessed early-return logic in `ApplyAttacksPerRound`.

2. **"@coderabbitai - Did you review this code?"** — A request directed at me to review a specific section.

3. **"@coderabbitai - This all feels like a code smell. Droid-specific functionality is now in a shared service and it probably should be in more droid-specific locations. Please review."** — A design concern about droid logic living in the shared `Stat.cs` service rather than a droid-specific location.

Additionally, my latest review left 2 open nitpick comments (non-blocking, but worth noting):

* **`EquipmentRestrictions.cs`**: The duplicated right/left-hand rebuild logic in the equip and unequip paths could be extracted into a shared helper to reduce drift risk.

* **`Stat.cs`**: A droid with no valid controller silently gets zero perks — this was flagged as worth confirming whether the behavior is intentional.

The owner's three threads are the blockers. I'd recommend addressing @zunath's concerns — particularly the design question about keeping droid-specific logic out of the shared Stat.cs service — before this is merged. Would you like me to do a deeper review on the code smell concern?
🧠 Learnings used

@coderabbitai Ignore the DM-Possessed creature request while we see what Zunath wants to do with that and confirm everything else is fine.

4a89401

This should have covered the other concerns.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 4, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check what commit 4a89401 changed
gh api repos/zunath/SWLOR_NWN/commits/4a89401e482984f013e9b5120b3b8e2826289ce0 --jq '.commit.message'
echo "---FILES CHANGED---"
gh api repos/zunath/SWLOR_NWN/commits/4a89401e482984f013e9b5120b3b8e2826289ce0 --jq '.files[] | {filename: .filename, additions: .additions, deletions: .deletions, patch: .patch}'

echo "---REVIEW THREADS---"
gh api graphql -f query='
{
  repository(owner: "zunath", name: "SWLOR_NWN") {
    pullRequest(number: 1956) {
      reviewThreads(first: 50) {
        nodes {
          isResolved
          comments(first: 5) {
            nodes {
              author { login }
              body
              url
            }
          }
        }
      }
    }
  }
}'

Length of output: 19726


@LazyTrain082 Confirmed — commit 4a89401 correctly addresses all the concerns I raised:

Code smell (Thread 3 — resolved): The refactoring is exactly as proposed. Stat.ApplyAttacksPerRound is now generic with an optional Func<PerkType, int> perkLevelResolver parameter, and all droid-specific logic has been moved to Droid.cs via BuildPerkLevelResolver and the new ApplyDroidAttacksPerRound public wrapper. No droid knowledge remains in Stat.

"Did you review this code?" (Thread 2 — already resolved): No action needed.

Silent zero perks nitpick: BuildPerkLevelResolver in Droid.cs now includes a GetIsObjectValid(controller) guard, so a droid with no valid controller gracefully returns 0 for all perks — intentional behavior per the project's conventions.

DM/DM-possessed thread (Thread 1 — unresolved): Noted, waiting on @zunath's decision as you requested. That's the only remaining open thread blocking merge.

The duplicated right/left-hand rebuild logic in EquipmentRestrictions.cs (equip vs. unequip paths) is still a non-blocking nitpick — happy to open a follow-up issue for that if you'd like to track it separately.


🧠 Learnings used
Learnt from: zunath
Repo: zunath/SWLOR_NWN PR: 1960
File: SWLOR.Game.Server/Feature/AbilityDefinition/Armor/ProvokeAbilityDefinition.cs:30-33
Timestamp: 2026-03-30T00:21:48.073Z
Learning: In SWLOR_NWN (C#), all droids (identified by ResRef via `Droid.IsDroid()`) are treated as player associates by design. Ownership verification is not needed when checking for droids in ability logic — if a creature is a droid (any summoned droid), it should be excluded from player-targeting abilities like Provoke. Do not suggest adding ownership-aware predicates for droid checks.

Learnt from: zunath
Repo: zunath/SWLOR_NWN PR: 1958
File: SWLOR.Game.Server/Feature/GuiDefinition/ViewModel/DMToolsViewModel.cs:770-770
Timestamp: 2026-04-04T17:02:24.442Z
Learning: Preference: For DM/admin tools in SWLOR.Game.Server, favor code-level concurrency guards over relying on manual DM coordination. In DMToolsViewModel, use placeable-local edit claims (DMTOOLS_EDITING_DM_ID/NAME/TS) with stale timeout and release on window close to prevent multi-DM overwrite.

Learnt from: LazyTrain082
Repo: zunath/SWLOR_NWN PR: 1956
File: SWLOR.Game.Server/Service/Stat.cs:0-0
Timestamp: 2026-04-04T20:32:39.507Z
Learning: In SWLOR_NWN (C#), a player-crafted droid can never be summoned without a droid controller. Therefore, no `GetIsObjectValid(controller)` guard is needed when calling `Droid.LoadDroidItemPropertyDetails(controller)` in paths that operate on player-owned crafted droids (e.g., `EquipmentRestrictions.cs`). Do not flag missing null/validity checks for the droid controller in these contexts.

@zunath
Copy link
Copy Markdown
Owner

zunath commented May 8, 2026

@codex review

@zunath
Copy link
Copy Markdown
Owner

zunath commented May 8, 2026

@coderabbitai - full review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 8, 2026

@zunath Sure! Triggering a fresh full review of PR #1956 now. 🐇

✅ Actions performed

Full review triggered.

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.

2 participants