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
Better upgrade visibility in editor UI #4491
base: master
Are you sure you want to change the base?
Conversation
I disagree with this being the default. I think fundamentally players should be fine with playing default organelle state for a while, and upgrades are mostly something different. So prompting immediately to modify a placed organelle doesn't make sense in a lot of cases. For example vacuoles, I don't think players would always want to specialize them, or for the pilus to convert it to deal toxin damage. There could be an option in the options menu to enable this but I don't think it should be on by default. |
Fair enough. Would automatically opening the right click menu be a sensible compromise? Or should I just change the text in the tooltip to read 'Right click to modify once placed' and leave it as it is otherwise? |
I'm not sure the menu opening would be that good flow either. At least it would require me extra clicks when building specific things to test. Also explicitly referring to right click is not a long term solution (I suggest using as neutral terminology as possible to avoid having a separate text for controller input): #4003 |
Right, but I'm just concerned that it's still obscuring information from the player if they aren't shown or told explicity how to modify the organelle. I'll remove the menu auto-opening for now so the tooltip and icon changes can at least go in. |
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.
Besides the translation conflict, I'd like to see a tiny bit of technical side clean up for this before merging.
@@ -260,6 +260,7 @@ private void UpdateOrganelleUnlockTooltips() | |||
var tooltip = GetSelectionTooltip(organelle.InternalName, "organelleSelection"); | |||
if (tooltip != null) | |||
{ | |||
tooltip.CanBeModified = organelle.CanBeModified; |
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.
Is this change actually required considering the code in SetupMicrobePartSelections
?
If this is needed, then I think it might warrant renaming this method to something like UpdateOrganelleTooltips
(as long as there isn't already a method with basically that name).
@@ -3892,6 +3892,10 @@ msgstr "Models" | |||
msgid "MODIFY" | |||
msgstr "Modify" | |||
|
|||
#: ../src/microbe_stage/editor/tooltips/SelectionMenuToolTip.tscn:163 | |||
msgid "MODIFY_ONCE_PLACED" | |||
msgstr "Modify once placed" |
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.
Is it the best that this text tells the player "hey, modify this" instead of being like "Can be modified after placing"? I feel like that's a bit off in terms of tone in a general characteristics info box listing.
margin_bottom = 19.0 | ||
custom_colors/font_color = Color( 1, 0.898039, 0.337255, 1 ) | ||
custom_fonts/font = ExtResource( 10 ) | ||
text = "MODIFY_ONCE_PLACED" |
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 text key would be better to have something like "PART_IS_MODIFIABLE"
@@ -2015,6 +2010,7 @@ private void SetupMicrobePartSelections() | |||
var control = (MicrobePartSelection)organelleSelectionButtonScene.Instance(); | |||
control.Locked = organelle.Unimplemented; | |||
control.PartIcon = organelle.LoadedIcon ?? throw new Exception("Organelle with no icon"); | |||
control.CanBeModified = organelle.CanBeModified; |
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.
Ah yeah, now I remember that this was separate from the tooltip. So this is also needed.
I guess we decided that for now showing the modifiable icon on the organelles that can be modified is fine. It'll be easy enough to make this an option / disable it once almost every single organelle can be modified.
We are currently in feature freeze until the next release. |
Brief Description of What This PR Does
Note this often results in the player having to undo twice if they want to remove a modifiable organelle due to #4091 which sadly still seems to be a bug.
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.