Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keymaster's Keep: add Monster Rancher 2 DX and add unittest #74

Merged
merged 5 commits into from
Feb 4, 2025

Conversation

Silvris
Copy link

@Silvris Silvris commented Feb 2, 2025

MR2DX support, nothing too crazy there.

Unittest simply just tests that the objectives of all games (optional + regular) return as string, which should hit all relevant code paths.

Copy link
Member

@nbrochu nbrochu left a comment

Choose a reason for hiding this comment

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

I would politely request to stick to games only for PRs, as highlighted in the contribution guidelines.

Updating the supported game docs ahead of time, while obviously done with good intentions, is not useful. I generate them once before release and like to look at the diff to do the credits. Also, it would show right away in the documents people refer to for the current release.

And the tests do not cover all paths since more and more games pivot on AP options to dynamically assemble the list of game objective templates. I simply don't want unit tests this early on something that will evolve quickly and where entire concepts might be thrown out the window with no notice. I'm a fan of unit tests when things are more stable, not as-you-go.

I do appreciate the implementation for a great game though. Thank you.

@nbrochu nbrochu merged commit 9ede99d into SerpentAI:kk Feb 4, 2025
9 of 10 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.

2 participants