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

Generalize challenges + classic/creative modes to scenarios #337

Merged
merged 56 commits into from
Jun 4, 2022
Merged

Conversation

byorgey
Copy link
Member

@byorgey byorgey commented May 21, 2022

Generalize challenges + various modes to all be "scenarios" which are described by .yaml files in data/scenarios.

  • Both challenges and classic/creative modes are now subsumed under the more general notion of "scenarios".
  • A scenario describes how to set up the world etc. when starting a game; all scenarios are stored in a .yaml file in data/scenarios.
  • "New game" menu item now lets the user choose a scenario.
  • Some small improvements to the way seeds are handled.

See #296. This will enable #35 and #25 .

byorgey added 30 commits April 3, 2022 00:08
Very open to suggestions/improvements/other ideas!
In preparation for generalizing challenges as outlined in #296.
The goal is to be able to load e.g. the classic game as a scenario.
There is no longer a separate special "classic" mode; it is just one
of the possible scenarios.
Also edit the classic scenario so it better matches what the baseRobot
function was doing.  Only remaining thing to do is to restore the
functionality of --run (which the baseRobot function was handling).
Dangerous because we almost never want to use it: we want to keep
around the old store and/or environment.
Note we now have what
#312 (comment) hinted
at, where `--run` works in any scenario, not just classic mode, since
we have merged classic and challenge modes into the more generic
"scenario" framework.

This still suffers from #333, but that bug already existed before, so
I'm leaving it to be fixed separately.
Also, do the loading inside `initGameState`, instead of passing a
weird monadic callback function parameter to it.
@byorgey byorgey marked this pull request as ready for review June 2, 2022 21:56
@byorgey
Copy link
Member Author

byorgey commented Jun 2, 2022

There's always more that can be done, but I think this is definitely ready for review & merge, and we can create issues to track the stuff remaining to do. See #296 for some related things still to do. Other than that, off the top of my head:

@byorgey
Copy link
Member Author

byorgey commented Jun 2, 2022

@xsebek , I know this is a large change, but if you get a chance I'd love your thoughts on this!

Copy link
Member

@xsebek xsebek left a comment

Choose a reason for hiding this comment

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

Overall feels very playable, great UI improvement @byorgey. 👍

I have a few suggestions, but some of them we can put into the parent Issue checklist and solve separately.

Better merge soon, or you will have to rebase on my PRs. 😄

continue $
s & uiState . uiMenu
.~ NewGameMenu (NE.fromList [mkScenarioList (s ^. gameState . scenarios)])
Tutorial -> continue $ s & uiState . uiMenu .~ TutorialMenu
Copy link
Member

Choose a reason for hiding this comment

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

Is this still well placed when the tutorials are in the New game?

Copy link
Member Author

@byorgey byorgey Jun 3, 2022

Choose a reason for hiding this comment

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

Well, my idea was as follows:

  • Once we create "meta-scenarios" which can consist of a sequence of other scenarios, The Tutorial will be such a thing. See Improvements to challenge mode #296 and Tutorial via challenge mode #25 .
  • We want the tutorial to be very prominently placed in the top-level menu, so someone just starting the game for the first time will immediately know how to start a tutorial. Selecting that top-level Tutorial menu item will automatically load the meta-scenario consisting of the sequence of tutorial scenarios.
  • However, it will still be possible to load individual tutorial scenarios through the New Game menu and selecting one manually.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, maybe we could leave a note that the tutorial is being slowly constructed over in the New game section?

In the meantime, you can play through a few tutorial parts over in the New game menu.

@byorgey
Copy link
Member Author

byorgey commented Jun 4, 2022

@xsebek thanks for the many great suggestions! I think I've addressed almost all of them now, what do you think?

@xsebek
Copy link
Member

xsebek commented Jun 4, 2022

LGTM 👍

@byorgey byorgey added the merge me Trigger the merge process of the Pull request. label Jun 4, 2022
@mergify mergify bot merged commit 8961d03 into main Jun 4, 2022
@mergify mergify bot deleted the scenario branch June 4, 2022 13:20
@byorgey byorgey mentioned this pull request Jun 4, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants