Upstream/joddy and dungeon clear fix#677
Conversation
Two fixes: 1. ConditionUtil.cs — add dungeon_clear to quest_clear group in CheckCode() (reuse Integers.Contains logic, matching dungeon room ID against condition codes) 2. DungeonFieldManager.cs — call ConditionUpdate on Clear state change (use DungeonId as codeLong, matching quest condition Codes.Integers) This allows quests with ConditionType.dungeon_clear (194) to complete after defeating a dungeon boss. First verified instance: quest 50001601 (dungeon room 25004001 -> map 2000331).
…le quests The quest_user_detected condition only checked questStates[0] and State 1 (Started) did not exclude CanComplete quests, causing a chain of failures: 1. DarkWindMovie.Ready (state=1) matched a completable quest, forcing the movie to replay and sending the player back to map 52000067 2. jordy.Ready (state=2) never triggered, so Joddy (NPC 11003220) never spawned in Dark Wind HQ (map 52000055) 3. Players could not accept follow-up quest 60100240, creating an infinite loop between maps 52000067 and 52000055 Fixes: - Iterate all questStates instead of only checking questStates[0] - State 1 now requires !CanComplete, strictly distinguishing it from State 2 Affects: Lv20 quest chain 60100235 -> 60100240 -> 60100245
📝 WalkthroughWalkthroughThe PR implements support for ChangesDungeon Clear Condition Support
Possibly Related PRs
Suggested Reviewers
Poem
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Maple2.Server.Game/Util/ConditionUtil.cs`:
- Line 89: The switch in ConditionUtil.CheckTarget is missing a case for
ConditionType.dungeon_clear causing target-matching to always fall through to
false; add a case ConditionType.dungeon_clear in CheckTarget and implement the
same target-evaluation logic used for ConditionType.quest_clear so that when a
dungeon_clear condition provides Target parameters the method returns the
target-match result (allowing ConditionUtil.Check's "target && code" evaluation
to succeed when appropriate).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ad62ff9b-67c5-4677-a7c9-e2d4b22ce17c
📒 Files selected for processing (3)
Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.csMaple2.Server.Game/Trigger/TriggerContext.Player.csMaple2.Server.Game/Util/ConditionUtil.cs
| case ConditionType.quest_accept: | ||
| case ConditionType.quest_clear_by_chapter: | ||
| case ConditionType.quest_clear: | ||
| case ConditionType.dungeon_clear: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for dungeon_clear condition definitions in metadata to verify if Target parameters are used.
# Search for dungeon_clear references in XML/JSON metadata files
rg -i -C5 'dungeon_clear' --type=xml --type=json
# Also search in any condition/quest metadata files
fd -e xml -e json | xargs rg -l -i 'condition.*dungeon'Repository: MS2Community/Maple2
Length of output: 47
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show relevant portions of ConditionUtil.cs around the referenced lines
FILE="Maple2.Server.Game/Util/ConditionUtil.cs"
echo "=== $FILE (around CheckCode / CheckTarget) ==="
nl -ba "$FILE" | sed -n '60,120p'
nl -ba "$FILE" | sed -n '260,360p'
# Broad search for dungeon_clear in code
echo "=== rg 'dungeon_clear' in repo ==="
rg -n --hidden --no-ignore -i 'dungeon_clear' .
# Search for ConditionType.dungeon_clear usage explicitly
echo "=== rg 'ConditionType\.dungeon_clear' in repo ==="
rg -n --hidden --no-ignore -i 'ConditionType\.dungeon_clear' .
# Safer metadata search (avoid xargs failure on empty input)
echo "=== metadata search for dungeon_clear in xml/json ==="
rg -n -C3 --hidden --no-ignore -i 'dungeon_clear' --type=xml --type=json . || true
echo "=== list files containing 'condition.*dungeon' for xml/json ==="
# Use fd to enumerate candidate files; only run rg if we actually find files.
CANDIDATES=$(fd --hidden --no-ignore -e xml -e json .)
if [ -n "$CANDIDATES" ]; then
echo "$CANDIDATES" | xargs -r rg -l -i --no-heading 'condition.*dungeon' || true
else
echo "No xml/json files found by fd (or fd returned nothing)."
fiRepository: MS2Community/Maple2
Length of output: 206
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Maple2.Server.Game/Util/ConditionUtil.cs"
echo "=== $FILE around CheckCode / CheckTarget ==="
awk 'NR>=60 && NR<=120 {printf "%6d\t%s\n", NR, $0}' "$FILE"
echo "=== $FILE around CheckTarget (260-360) ==="
awk 'NR>=260 && NR<=360 {printf "%6d\t%s\n", NR, $0}' "$FILE"
echo "=== rg 'dungeon_clear' in repo (limit output to hits) ==="
rg -n --hidden --no-ignore -i 'dungeon_clear' . | head -n 200
echo "=== rg 'ConditionType\\.dungeon_clear' in repo ==="
rg -n --hidden --no-ignore -i 'ConditionType\.dungeon_clear' . | head -n 200
echo "=== metadata search for dungeon_clear in xml/json ==="
# Avoid xargs empty-input issues by only running rg if we find candidates.
CANDIDATES=$(fd --hidden --no-ignore -e xml -e json .)
if [ -n "$CANDIDATES" ]; then
echo "$CANDIDATES" | xargs -r rg -n -i -C2 'dungeon_clear' || true
else
echo "No xml/json files found by fd."
fiRepository: MS2Community/Maple2
Length of output: 9025
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Maple2.Server.Game/Util/ConditionUtil.cs"
echo "=== method signature locations ==="
rg -n "CheckTarget|CheckCode" "$FILE"
echo "=== ConditionUtil.cs around CheckTarget definition ==="
# Print wider range around the first match to include signature + body
LINE_CHECKTARGET=$(rg -n "CheckTarget" "$FILE" | head -n1 | cut -d: -f1)
START=$((LINE_CHECKTARGET-60))
END=$((LINE_CHECKTARGET+160))
awk -v s=$START -v e=$END 'NR>=s && NR<=e {printf "%6d\t%s\n", NR, $0}' "$FILE"
echo "=== occurrences of .Target or Target in ConditionUtil.cs ==="
rg -n "\bTarget\b|\.Target\b" "$FILE" | head -n 200
echo "=== ConditionMetadata definition (search) ==="
rg -n "class ConditionMetadata|struct ConditionMetadata|ConditionMetadata" Maple2.Server.Game/Util/ConditionUtil.cs Maple2.Server.Game -S || true
echo "=== occurrences of CheckTarget call sites ==="
rg -n "CheckTarget\\(" Maple2.Server.Game -S || trueRepository: MS2Community/Maple2
Length of output: 18361
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="Maple2.Server.Game/Util/ConditionUtil.cs"
echo "=== CheckTarget full body (approx 160-340) ==="
awk 'NR>=160 && NR<=340 {printf "%6d\t%s\n", NR, $0}' "$FILE"
echo "=== lines with quest_clear / dungeon_clear inside ConditionUtil.cs ==="
rg -n "quest_clear|dungeon_clear" "$FILE"
echo "=== non-.cs occurrences of dungeon_clear in repo ==="
rg -n -i --glob '!*.cs' 'dungeon_clear' . | head -n 50 || true
echo "=== non-.cs occurrences of quest_clear in repo ==="
rg -n -i --glob '!*.cs' 'quest_clear' . | head -n 50 || trueRepository: MS2Community/Maple2
Length of output: 9629
Add ConditionType.dungeon_clear to CheckTarget
ConditionUtil.CheckCode includes case ConditionType.dungeon_clear:, but CheckTarget’s switch (conditionType) has no dungeon_clear case and falls back to return false (end of method). If a dungeon_clear condition provides Target parameters, ConditionUtil.Check evaluates target && code, so the condition can fail even when the code part matches (similar to how quest_clear is handled in CheckTarget).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Maple2.Server.Game/Util/ConditionUtil.cs` at line 89, The switch in
ConditionUtil.CheckTarget is missing a case for ConditionType.dungeon_clear
causing target-matching to always fall through to false; add a case
ConditionType.dungeon_clear in CheckTarget and implement the same
target-evaluation logic used for ConditionType.quest_clear so that when a
dungeon_clear condition provides Target parameters the method returns the
target-match result (allowing ConditionUtil.Check's "target && code" evaluation
to succeed when appropriate).
Two fixes for quest progression:
1. dungeon_clear condition fix (quest 50001601)
ConditionType.dungeon_clear(194) was never satisfied after dungeon boss kills. Added the condition to the quest_clear handler and emitConditionUpdateon dungeon state change.2. Joddy not appearing (Lv20 quest chain)
QuestUserDetectedState1 incorrectly matched completable quests, causing DarkWindMovie to replay infinitely and Joddy (11003220) to never spawn. Players stuck between maps 52000067 ↔ 52000055.Summary by CodeRabbit