Skip to content

Upstream/joddy and dungeon clear fix#677

Merged
AngeloTadeucci merged 2 commits into
MS2Community:masterfrom
AnikiKun:upstream/joddy-and-dungeon-clear-fix
Jun 11, 2026
Merged

Upstream/joddy and dungeon clear fix#677
AngeloTadeucci merged 2 commits into
MS2Community:masterfrom
AnikiKun:upstream/joddy-and-dungeon-clear-fix

Conversation

@AnikiKun

@AnikiKun AnikiKun commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

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 emit ConditionUpdate on dungeon state change.

2. Joddy not appearing (Lv20 quest chain)

QuestUserDetected State1 incorrectly matched completable quests, causing DarkWindMovie to replay infinitely and Joddy (11003220) to never spawn. Players stuck between maps 52000067 ↔ 52000055.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed dungeon completion tracking to properly update all players when a dungeon is cleared.
    • Fixed quest state detection to correctly evaluate all quest progression states.
    • Added proper support for dungeon clear condition evaluation.

AnikiKun added 2 commits June 12, 2026 02:31
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
@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR implements support for dungeon_clear condition type by extending condition evaluation infrastructure and emitting condition updates when dungeons clear. Three files are modified: condition matching logic is added to ConditionUtil and improved in TriggerContext, and event emission is added to DungeonFieldManager.

Changes

Dungeon Clear Condition Support

Layer / File(s) Summary
Condition matching infrastructure
Maple2.Server.Game/Util/ConditionUtil.cs, Maple2.Server.Game/Trigger/TriggerContext.Player.cs
CheckCode switch adds dungeon_clear case for range/integers matching; QuestUserDetected now iterates all quest states to determine player quest state match instead of checking only the first state.
Dungeon clear event trigger
Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs
When DungeonRoomRecord.State transitions to Clear, ChangeState sends a ConditionUpdate with ConditionType.dungeon_clear and codeLong: DungeonId to each player before other clear-handling logic.

Possibly Related PRs

  • MS2Community/Maple2#310: Similarly extends ConditionUtil.CheckCode to add a new ConditionType case (music_play_ensemble) for condition matching.

Suggested Reviewers

  • Zintixx

Poem

🐰 Dungeons cleared with pride so bright,
Conditions sent to mark the fight,
Quest states checked—each one, each way,
A rabbit's gift for dungeon day! 🏰


🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Upstream/joddy and dungeon clear fix' clearly refers to two specific fixes mentioned in the PR objectives: the Joddy NPC quest progression issue and the dungeon_clear condition fix. It accurately summarizes the main changes.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between ee532da and 29e8ffc.

📒 Files selected for processing (3)
  • Maple2.Server.Game/Manager/Field/FieldManager/DungeonFieldManager.cs
  • Maple2.Server.Game/Trigger/TriggerContext.Player.cs
  • Maple2.Server.Game/Util/ConditionUtil.cs

case ConditionType.quest_accept:
case ConditionType.quest_clear_by_chapter:
case ConditionType.quest_clear:
case ConditionType.dungeon_clear:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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)."
fi

Repository: 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."
fi

Repository: 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 || true

Repository: 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 || true

Repository: 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).

@AngeloTadeucci AngeloTadeucci merged commit a07f56d into MS2Community:master Jun 11, 2026
3 checks passed
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.

3 participants