-
Notifications
You must be signed in to change notification settings - Fork 56
Add available recipes help #441
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
Conversation
0704abc
to
beb7ddb
Compare
I'd be happy to implement this differently, but that seems to work as expected. |
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 looks really cool so far! A few general comments:
- At the start of the game, when there are no recipes available, pressing F2 seems to open a tiny empty dialog box in the middle of the screen.
- I think recipes should become available as soon as we know about an entity, not just as soon as we actually have one. When I started a new classic game and sent a robot to scan a tree, no recipes showed up. But when I sent a robot to actually fetch a tree, then the recipe showed up.
- The recipes don't seem to be updated when I obtain an entity via the
create
command. (See, this is exactly what I meant about forgetting to callupdateAvailableRecipes
in all the right places.) - I think the recipe dialog should put a blank line between recipes to make it easier to tell them apart.
- Recipes seem to be stretched across the entire available horizontal space which makes them difficult to read.
- What will happen when the available recipes dialog gets bigger than the screen?
- I'd like some way in the available recipes dialog to tell which recipes are new/unread. Maybe a horizontal red line separating the new ones from the old ones?
1118639
to
143d8e0
Compare
Thank you for the detailed review @byorgey , really appreciate it!
The dialog is disabled when no recipes are available: 368f2c3.
The list is updated on scan too, but I wonder if that is desirable: scan may be used in bulk, and I think it would be better to wait for an actual entity acquisition. 66574c4
That's odd, it should have worked... Perhaps you were missing an installed devices?
Done in e919ef0
Fixed in 44576ac
143d8e0 adds a viewport to enable scrolling.
Done in e919ef0, though I wasn't able to change the line color. |
Hmm, I think I disagree. It's a bit difficult to scan all that many things at once in the beginning of the game, and knowing there are things I can make or do with the entities I just learned about is great incentive to go get some. |
Alright, that works for me. Then I think the PR is now ready for review again, I believe it's working as expected. |
Thanks for addressing all those things, this is coming along nicely. Unfortunately it still doesn't seem to be working quite right for me. I got a tree and it told me about a recipe to make branch + log; I made branches and it told me about the recipe to make a branch predictor. So far so good.... except that since I also obtained a I then switched into creative mode and did I still find the recipes are hard to read because they are very wide. Can we add some blank space on either side of the recipes (i.e. with |
I see, the update only considered the given Make entity, not the recipe by-products. This should now be fixed with 155c64d (along with the padding). For the recipes enabled by install-able entities such as the |
Works great now! And looks much better with the padding. I personally would also prefer a blank line between the top of the dialog box and the first recipe, but now we're definitely just getting into the territory of personal preferences and I'm happy to merge it either way. 😄
Ah, I see the confusion. Furnaces don't actually need to be installed, nor do any entities listed as a required "catalyst" (i.e. centered at the top of a recipe), at least not as far as the recipe is concerned. You certainly can |
One other thing: I think it would be nice if there was a slightly more prominent way to alert the player that they have new recipes available. A single asterisk showing up is pretty easy to miss if your eyes are looking somewhere else. I like the idea (I think @xsebek mentioned this?) of highlighting |
That makes sense, today I learned furnaces do not have to be installed :-) In the last change I've also updated the styling as you suggested, though only the Thank you for helping me figure all that out! |
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.
All looks great! I like highlighting just the [F2]
, that seems noticeable without being too in-your-face. Thanks for your work on this great feature, I think it represents a huge jump in the playability/discoverability of the game.
As for the menu getting too crowded, I think @xsebek suggested moving all the [Fn]
key shortcuts to the top left of the world, which is something we might consider. But that can wait for another PR!
@TristanCacqueray feel free to merge this unless there are any last-minute tweaks you want to make! |
Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Restyled.io <[email protected]>
4f89f8a
to
bebd501
Compare
This change adds a new help panel with the list of recently acquired recipes.
Fixes #436