Skip to content

fix: prevent self-referential EventAchievement infinite loops#4908

Open
wescopeland wants to merge 4 commits into
RetroAchievements:masterfrom
wescopeland:event-ach-inf-loop-fix
Open

fix: prevent self-referential EventAchievement infinite loops#4908
wescopeland wants to merge 4 commits into
RetroAchievements:masterfrom
wescopeland:event-ach-inf-loop-fix

Conversation

@wescopeland
Copy link
Copy Markdown
Member

@wescopeland wescopeland commented May 20, 2026

@wescopeland wescopeland requested a review from a team May 20, 2026 21:29
) {
$fail('Source achievement cannot be the same as the event achievement.');
}
};
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would we ever want to allow any event achievement to depend on another event achievement? Should the restriction just be "source achievement cannot be event achievement"?

Otherwise, we could still end up with an infinite loop if any achievement in the chain pointed back to the start node: A -> B -> C -> A

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good point.

I've reverted most of the original implementation in latest (let's just treat it as a data issue if it happens in prod somehow) and tightened this rule.

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.

2 participants