fix: spellbooks can have charges ≠ 3 (#38769)

* fix: spellbooks can have charges ≠ 3

* refactor: just make setting MaxCharges an api thing

* refactor: don't auto-add LimitedCharges

* Small stuff

---------

Co-authored-by: Princess Cheeseballs <66055347+Pronana@users.noreply.github.com>

Content.Shared/Charges/Components/LimitedChargesComponent.cs

@@ -16,7 +16,7 @@ public sealed partial class LimitedChargesComponent : Component
     /// <summary>
     ///     The max charges this action has.
     /// </summary>
-    [DataField, AutoNetworkedField, Access(Other = AccessPermissions.Read)]
+    [DataField, AutoNetworkedField]
     public int MaxCharges = 3;
 
     /// <summary>

Content.Shared/Charges/Systems/SharedChargesSystem.cs

@@ -94,6 +94,12 @@ public abstract class SharedChargesSystem : EntitySystem
     /// <summary>
     /// Adds the specified charges. Does not reset the accumulator.
     /// </summary>
+    /// <param name="action">
+    /// The action to add charges to. If it doesn't have <see cref="LimitedChargesComponent"/>, it will be added.
+    /// </param>
+    /// <param name="addCharges">
+    /// The number of charges to add. Can be negative. Resulting charge count is clamped to [0, MaxCharges].
+    /// </param>
     public void AddCharges(Entity<LimitedChargesComponent?, AutoRechargeComponent?> action, int addCharges)
     {
         if (addCharges == 0)
@@ -170,9 +176,21 @@ public abstract class SharedChargesSystem : EntitySystem
         Dirty(action);
     }
 
+    /// <summary>
+    /// Set the number of charges an action has.
+    /// </summary>
+    /// <param name="action">The action in question</param>
+    /// <param name="value">
+    /// The number of charges. Clamped to [0, MaxCharges].
+    /// </param>
+    /// <remarks>
+    /// This method doesn't implicitly add <see cref="LimitedChargesComponent"/>
+    /// unlike some other methods in this system.
+    /// </remarks>
     public void SetCharges(Entity<LimitedChargesComponent?> action, int value)
     {
-        action.Comp ??= EnsureComp<LimitedChargesComponent>(action.Owner);
+        if (!Resolve(action, ref action.Comp))
+            return;
 
         var adjusted = Math.Clamp(value, 0, action.Comp.MaxCharges);
 
@@ -186,6 +204,31 @@ public abstract class SharedChargesSystem : EntitySystem
         Dirty(action);
     }
 
+    /// <summary>
+    /// Sets the maximum charges of a given action.
+    /// </summary>
+    /// <param name="action">The action being modified.</param>
+    /// <param name="value">The new maximum charges of the action. Clamped to zero.</param>
+    /// <remarks>
+    /// Does not change the current charge count, or adjust the
+    /// accumulator for auto-recharge. It also doesn't implicitly add
+    /// <see cref="LimitedChargesComponent"/> unlike some other methods
+    /// in this system.
+    /// </remarks>
+    public void SetMaxCharges(Entity<LimitedChargesComponent?> action, int value)
+    {
+        if (!Resolve(action, ref action.Comp))
+            return;
+
+        // You can't have negative max charges (even zero is a bit goofy but eh)
+        var adjusted = Math.Max(0, value);
+        if (action.Comp.MaxCharges == adjusted)
+            return;
+
+        action.Comp.MaxCharges = adjusted;
+        Dirty(action);
+    }
+
     /// <summary>
     /// The next time a charge will be considered to be filled.
     /// </summary>

Content.Shared/Magic/SpellbookSystem.cs

@@ -1,5 +1,6 @@
 using Content.Shared.Actions;
-using Content.Shared.Actions.Components;
+using Content.Shared.Actions.Components;
+using Content.Shared.Charges.Components;
 using Content.Shared.Charges.Systems;
 using Content.Shared.DoAfter;
 using Content.Shared.Interaction.Events;
@@ -29,14 +30,19 @@ public sealed class SpellbookSystem : EntitySystem
     {
         foreach (var (id, charges) in ent.Comp.SpellActions)
         {
-            var spell = _actionContainer.AddAction(ent, id);
+            var action = _actionContainer.AddAction(ent, id);
-            if (spell == null)
+            if (action is not { } spell)
                 continue;
 
             // Null means infinite charges.
             if (charges is { } count)
-                _sharedCharges.SetCharges(spell.Value, count);
+            {
-            ent.Comp.Spells.Add(spell.Value);
+                EnsureComp<LimitedChargesComponent>(spell, out var chargeComp);
+                _sharedCharges.SetMaxCharges((spell, chargeComp), count);
+                _sharedCharges.SetCharges((spell, chargeComp), count);
+            }
+
+            ent.Comp.Spells.Add(spell);
         }
     }
 
@@ -75,9 +81,13 @@ public sealed class SpellbookSystem : EntitySystem
             foreach (var (id, charges) in ent.Comp.SpellActions)
             {
                 EntityUid? actionId = null;
-                if (_actions.AddAction(args.Args.User, ref actionId, id)
+                if (!_actions.AddAction(args.Args.User, ref actionId, id)
-                    && charges is { } count) // Null means infinite charges
+                    || charges is not { } count // Null means infinite charges
-                    _sharedCharges.SetCharges(actionId.Value, count);
+                    || !TryComp<LimitedChargesComponent>(actionId, out var chargeComp))
+                    continue;
+
+                _sharedCharges.SetMaxCharges((actionId.Value, chargeComp), count);
+                _sharedCharges.SetCharges((actionId.Value, chargeComp), count);
             }
         }