-
-
Notifications
You must be signed in to change notification settings - Fork 521
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
Tutorial Improvements #5938
base: master
Are you sure you want to change the base?
Tutorial Improvements #5938
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't play with the new tutorial yet, so this is just an architectural review for now. I think the HUD control from individual tutorials needs a bit more work, but other than that other problems were just minor.
public const float MICROBE_MOVEMENT_EXPLAIN_TUTORIAL_DELAY_CONTROLLER = 1.0f; | ||
public const float MICROBE_MOVEMENT_TUTORIAL_REQUIRE_DIRECTION_PRESS_TIME = 2.2f; | ||
public const float MICROBE_MOVEMENT_TUTORIAL_REQUIRE_DIRECTION_PRESS_TIME = 1.6f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should not be reduced until we have a popup for the player to pick their preferred movement mode.
public const float TUTORIAL_ENTITY_POSITION_UPDATE_INTERVAL = 0.2f; | ||
public const float GLUCOSE_TUTORIAL_TRIGGER_ENABLE_FREE_STORAGE_SPACE = 0.14f; | ||
public const float GLUCOSE_TUTORIAL_TRIGGER_ENABLE_FREE_STORAGE_SPACE = 0.03f; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this cause problems with the player being able to find glucose way too soon? I've balanced the tutorial so that at most you need to spend like one second cluelessly in the glucose cloud wondering if it is working or not. With less storage space the player may need to spend a lot longer inside a cloud attempting to absorb it even when their storage is full.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable could be put back probably given the other tutorials, but there's a new change later to set the glucose manually so that there's always enough free space to finish that tutorial phase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I did see that code later. To me it doesn't seem exactly good that the player data is passed to the tutorial like that to be modified, that's part of the major architectural problem I think exists here I commented on in my review.
|
||
if (!HasBeenShown && data.EntityPosition.HasValue && CanTrigger && !overallState.TutorialActive()) | ||
{ | ||
// force player microbe to have the right amount of Glucose missing | ||
compounds.TakeCompound(Compound.Glucose, compounds.GetCapacityForCompound(Compound.Glucose)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see this change. I personally would prefer to keep the approach where the tutorial only triggers after the player can reasonable be expected to have enough missing storage space.
I might be convinced to see this is better but I think 0.2 glucose buffer is pretty low safety margin and we should not make the player more likely to die during the tutorial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was toying with the idea of having a check to make sure that the glucose never falls below 0.1 during this stage, but I didn't for two reason:
- it would be complex and likely error prone messaging code (and you said you wanted small incremental changes)
- This is immediately after the player reads a message that says "if you don't have ATP you die". 5-10 seconds into the tutorial, I'm more afraid of players wondering what they are supposed to do after 15 seconds of nothing happening and getting lost in menus than dying from the thing the tutorial was talking about killing you and having to respawn
It's all subjective, but I think a key problem in the early tutorial is a lack of responsiveness. All the streamers I watched are eager to do things and see results, and our first tutorials don't do that very well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would actually suggest a pretty simple code addition to the _Process method of the stage: check if the tutorial state is active (i.e. a tutorial is open) and the compounds panel is not open (and the player has not gotten to the editor yet, this should prevent players exploiting it), if so add glucose to the player.
The movement tutorial is meant to last roughly 30 seconds for the slower players. So all tutorials need to be timed so that it still leaves enough time for the movement tutorial to be shown at a relaxed pace.
@@ -21,8 +21,11 @@ public override bool CheckEvent(TutorialState overallState, TutorialEventType ev | |||
{ | |||
case TutorialEventType.MicrobePlayerEnterSunlightPatch: | |||
{ | |||
MicrobeHUD hud = ((HUDEventArgs)args).HUD; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is pretty dirty to pass the HUD like this.
Also the environment panel has to be shown for temperature change with thermosynthesis.
I don't immediately have a good idea here, but I think that some kind of system like "wants pause" going through the tutorial state would be better. Maybe some kind of event system where the TutorialState offers an event callback any active HUD registers to, and individual tutorials can then send events through like "EnvironmentPanelRequired"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the args is a little abused in our tutorials. If you have a better pattern you prefer, however, you'll probably need to explain to me how it's going to work...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did lay out a rough idea. After having it stew in my head for a while, I think it is probably the best option, so adding a method in TutorialState to trigger a tutorial event that is listened to by active HUD instances, would be the way to go in my opinion. Then the tutorials can send events like "compounds panel should be visible" etc. and the relevant HUD then reacts to it.
if (!HasBeenShown && CanTrigger && !overallState.TutorialActive()) | ||
OrganelleDefinition placedOrganelle = ((OrganellePlacedEventArgs)args).Definition; | ||
|
||
if (!HasBeenShown && CanTrigger && !overallState.TutorialActive() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really think we need to tutorialize the undo feature earlier than when placing the nucleus.
We could have another tutorial reminding about undo when placing the nucleus (if the player ATP balance / resource balance is not good). But I don't think we should give up the undo tutorial, because back in the dark ages people used to often ask on our discord how to undo. And I don't want to go back to that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't the undo tutorial waiting until the player is ready for the nucleus, this is the editor tutorial telling the player to put a nucleus on first, then explaining why that would be a terrible idea and explaining the undo button.
It is entirely possible for a player to ignore this advice and skip this phase by doing different modifications, but I opted to not force the player to read more walls of text if they are aren't ignoring instructions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then the player is allowed to skip like half of the hands on tutorial for the first editor cycle, which I don't think is good...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the redo tutorial had a pretty nice flow to it and it didn't add too much time. And it synergizes really well with the general undo tutorial that is early in the process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think anyone sees an undo button and can't guess what the button next to it does, and every time you interrupt flow to read a box of text is another couple players who get bored and turn off the tutorial before the important parts.
@@ -62,12 +62,16 @@ public void ReuseEvent(bool usesScreenRelativeMovement, Vector3 movementDirectio | |||
|
|||
public class EntityPositionEventArgs : TutorialEventArgs | |||
{ | |||
public EntityPositionEventArgs(Vector3? position) | |||
public EntityPositionEventArgs(Vector3? position, CompoundBag compounds, MicrobeHUD hud) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which compounds are these, the entity's or the player's? I think that needs naming clarification. Also I'd totally remove the HUD as I think it just doesn't fit at all into the idea of event args. At most I think a hack with the sender of the event could be done but even that seems not very clean design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll rename the compounds. As for the HUD I have no attachment to this pattern, I just didn't see a cleaner way to send it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After reading the code it seems like it is the current player's data, which kind of doesn't fit logically as the position is for another entity and the player data is just tacked on... So I would really prefer if that data was also stripped so that we follow more separation of concerns (disallowing the tutorial from directly messing with the player state).
Brief Description of What This PR Does
Updates a few sequences and a lot of text of tutorials, aiming to avoid leading the player down "rabbit holes" of the more complicated bits of the game, improve some clarity, optimize use of limited player attention, and add a little bit a scientific narrative poetry to do some story-telling while trying to explain the game.
Related Issues
Progress Checklist
Note: before starting this checklist the PR should be marked as non-draft.
break existing features:
https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
(this is important as to not waste the time of Thrive team
members reviewing this PR)
styleguide.
Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.