Skip to content

Commit b257a15

Browse files
fix(plugin): address code review feedback on skill documentation
- Add null-conditional operator for TakeDamage in DoT pattern - Change ReadOnlySpan to array in async wave spawner (ref structs invalid) - Add Reset() methods to BulletLifetimeState and TypewriterState - Add Cleanup/Dispose methods to ComboSystem, RegenState, TooltipTrigger - Fix TypewriterState to use static lambda with state parameter - Ensure all JActions are properly disposed in long-running patterns Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: JasonXuDeveloper - 傑 <jason@xgamedev.net>
1 parent e0f4c11 commit b257a15

File tree

2 files changed

+77
-17
lines changed

2 files changed

+77
-17
lines changed

.claude-plugin/skills/game-patterns/SKILL.md

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ public sealed class ComboState
116116
public JAction ResetAction;
117117
}
118118

119-
public sealed class ComboSystem
119+
public sealed class ComboSystem : IDisposable
120120
{
121121
private readonly ComboState _state = new();
122122

@@ -125,6 +125,7 @@ public sealed class ComboSystem
125125
public void OnHit()
126126
{
127127
_state.ResetAction?.Cancel();
128+
_state.ResetAction?.Dispose();
128129
_state.Count++;
129130

130131
_state.ResetAction = JAction.Create()
@@ -133,6 +134,16 @@ public sealed class ComboSystem
133134

134135
_ = _state.ResetAction.ExecuteAsync();
135136
}
137+
138+
/// <summary>
139+
/// Call from OnDestroy to prevent callbacks on destroyed objects.
140+
/// </summary>
141+
public void Dispose()
142+
{
143+
_state.ResetAction?.Cancel();
144+
_state.ResetAction?.Dispose();
145+
_state.ResetAction = null;
146+
}
136147
}
137148
```
138149

@@ -206,13 +217,20 @@ public sealed class RegenState
206217
public float Max;
207218
public float PerTick;
208219
public JAction Action;
220+
221+
public void Cleanup()
222+
{
223+
Action?.Cancel();
224+
Action?.Dispose();
225+
Action = null;
226+
}
209227
}
210228

211229
public static class RegenSystem
212230
{
213231
public static void Start(RegenState state, float hpPerSecond)
214232
{
215-
state.Action?.Cancel();
233+
state.Cleanup();
216234
state.PerTick = hpPerSecond * 0.1f;
217235

218236
state.Action = JAction.Create()
@@ -225,7 +243,10 @@ public static class RegenSystem
225243
_ = state.Action.ExecuteAsync();
226244
}
227245

228-
public static void Stop(RegenState state) => state.Action?.Cancel();
246+
/// <summary>
247+
/// Stop regeneration. Call Cleanup() from OnDestroy to fully dispose.
248+
/// </summary>
249+
public static void Stop(RegenState state) => state.Cleanup();
229250
}
230251
```
231252

@@ -283,6 +304,12 @@ public sealed class BulletLifetimeState
283304
{
284305
public Bullet Bullet;
285306
public BulletManager Manager;
307+
308+
public void Reset()
309+
{
310+
Bullet = null;
311+
Manager = null;
312+
}
286313
}
287314

288315
public static async UniTaskVoid FireWithLifetime(
@@ -301,11 +328,10 @@ public static async UniTaskVoid FireWithLifetime(
301328

302329
using var action = await JAction.Create()
303330
.Delay(lifetime)
304-
.Do(static s => s.Manager.Return(s.Bullet), state)
331+
.Do(static s => s.Manager?.Return(s.Bullet), state)
305332
.ExecuteAsync();
306333

307-
state.Bullet = null;
308-
state.Manager = null;
334+
state.Reset();
309335
JObjectPool.Shared<BulletLifetimeState>().Return(state);
310336
}
311337
```
@@ -319,14 +345,24 @@ public sealed class TooltipState
319345
public JAction Action;
320346
public Action ShowCallback;
321347
public Action HideCallback;
348+
349+
public void Reset()
350+
{
351+
Action = null;
352+
ShowCallback = null;
353+
HideCallback = null;
354+
}
322355
}
323356

324-
public sealed class TooltipTrigger
357+
public sealed class TooltipTrigger : IDisposable
325358
{
326359
private readonly TooltipState _state = new();
327360

328361
public void OnPointerEnter(Action show, Action hide)
329362
{
363+
_state.Action?.Cancel();
364+
_state.Action?.Dispose();
365+
330366
_state.ShowCallback = show;
331367
_state.HideCallback = hide;
332368

@@ -340,20 +376,43 @@ public sealed class TooltipTrigger
340376
public void OnPointerExit()
341377
{
342378
_state.Action?.Cancel();
379+
_state.Action?.Dispose();
380+
_state.Action = null;
343381
_state.HideCallback?.Invoke();
344382
}
383+
384+
/// <summary>
385+
/// Call from OnDestroy to prevent callbacks on destroyed objects.
386+
/// </summary>
387+
public void Dispose()
388+
{
389+
_state.Action?.Cancel();
390+
_state.Action?.Dispose();
391+
_state.Reset();
392+
}
345393
}
346394
```
347395

348-
### Typewriter Effect (Zero-GC with StringBuilder Pool)
396+
### Typewriter Effect (with StringBuilder Pool)
349397
```csharp
350398
public sealed class TypewriterState
351399
{
352400
public string FullText;
353401
public int CurrentIndex;
354402
public Action<string> OnUpdate;
403+
public StringBuilder Builder;
404+
405+
public void Reset()
406+
{
407+
FullText = null;
408+
CurrentIndex = 0;
409+
OnUpdate = null;
410+
Builder = null;
411+
}
355412
}
356413

414+
// Note: This pattern uses a closure for simplicity. For high-frequency usage,
415+
// consider wrapping StringBuilder in the state class to achieve full zero-GC.
357416
public static async UniTask TypeText(string content, float charDelay, Action<string> onUpdate)
358417
{
359418
var state = JObjectPool.Shared<TypewriterState>().Rent();
@@ -362,26 +421,27 @@ public static async UniTask TypeText(string content, float charDelay, Action<str
362421
state.FullText = content;
363422
state.CurrentIndex = 0;
364423
state.OnUpdate = onUpdate;
424+
state.Builder = sb;
365425

366426
using var action = await JAction.Create()
367427
.Repeat(
368-
() =>
428+
static s =>
369429
{
370-
if (state.CurrentIndex < state.FullText.Length)
430+
if (s.CurrentIndex < s.FullText.Length)
371431
{
372-
sb.Append(state.FullText[state.CurrentIndex++]);
373-
state.OnUpdate?.Invoke(sb.ToString());
432+
s.Builder.Append(s.FullText[s.CurrentIndex++]);
433+
s.OnUpdate?.Invoke(s.Builder.ToString());
374434
}
375435
},
436+
state,
376437
count: content.Length,
377438
interval: charDelay)
378439
.ExecuteAsync();
379440

380441
sb.Clear();
381442
JObjectPool.Shared<StringBuilder>().Return(sb);
382443

383-
state.FullText = null;
384-
state.OnUpdate = null;
444+
state.Reset();
385445
JObjectPool.Shared<TypewriterState>().Return(state);
386446
}
387447
```

.claude-plugin/skills/jaction/SKILL.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public static async UniTaskVoid ApplyDoT(IDamageable target, float damage, int t
168168

169169
using var action = await JAction.Create()
170170
.Repeat(
171-
static s => s.Target.TakeDamage(s.DamagePerTick),
171+
static s => s.Target?.TakeDamage(s.DamagePerTick),
172172
state,
173173
count: ticks,
174174
interval: interval)
@@ -182,9 +182,9 @@ public static async UniTaskVoid ApplyDoT(IDamageable target, float damage, int t
182182

183183
### Wave Spawner (Async)
184184
```csharp
185-
public async UniTask RunWaves(ReadOnlySpan<WaveConfig> waves)
185+
public async UniTask RunWaves(WaveConfig[] waves)
186186
{
187-
foreach (ref readonly var wave in waves)
187+
foreach (var wave in waves)
188188
{
189189
using var action = await JAction.Create()
190190
.Do(() => UI.ShowWaveStart(wave.Number))

0 commit comments

Comments
 (0)