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

Make ability to remove vehicle parts more obvious #40087

Merged
merged 3 commits into from
May 5, 2020

Conversation

procrustesmethod
Copy link
Contributor

@procrustesmethod procrustesmethod commented May 3, 2020

Summary

Purpose of change

I've seen newer players get confused when trying to remove parts from a vehicle. In OR statements (i.e. lifting 1 OR strength 1), the only colors for the requirements were red or green. So if you didn't have a forklift but weren't severely cripped, LIFTING would be red while STRENGTH would be green.

However, when installing parts, or crafting stuff, the text is red when all requirements are unfulfilled, and grey/green when some requirements are unfulfilled/fulfilled (respectively.) Thus some people, when they see red, think they aren't fulfilling that requirement & they'll build a forklift.

This change is to keep the interface consistent and resolve some confusion.

Describe the solution

Change the code so OR statements in the additional requirements, when removing stuff from a vehicle, follow the same color scheme that the install menu and crafting menu do.

(AFAIK these OR statements only appear under "additional requirements". If I'm wrong please let me know!)

Describe alternatives you've considered

Not changing the color scheme.

Testing

I tried removing a seatbelt before and after my changes. Color scheme was satisfactorily changed.

BEFORE:

old_remove

AFTER:

new_remove

AFTER: ALL UNFULFILLED

new_fail

Additional context

To clarify:
Here at the conditions for skill/tool required & additional requirements:
Do you have the skills? Y: green, N: red
Do you have the tools? Y: green, N:red

Additional requirements:
Say the first requirement is P , second is Q. Both boolean.
P ^ Q -> Green & green
!P ^ !Q -> Red & Red
P ^ !Q -> Green & Grey
!P ^ Q -> Grey & Green

Ignore any inconsistencies w.r.t tools/skills required in screenshots above.

@wapcaplet
Copy link
Contributor

wapcaplet commented May 3, 2020

Good work - this part of the UI has confused me as well. Thanks for including before and after screenshots too; those make it a lot easier to see what has changed in-game.

I see a lot of unrelated case statements in the file were reformatted along with your changes (indentation reduced):

image

If you didn't make those changes yourself, it is possible your editor/IDE may be doing it automatically. Unfortunately, these reformattings make it harder to see what part of the code you've actually changed, which is the reason for the astyle failure in the continuous integration checks (to keep indentation and line-wrapping consistent through revisions). You can clean these up pretty easily by running make astyle in your branch, then committing and pushing the changes.

@procrustesmethod
Copy link
Contributor Author

Thanks for your help! I don't think my editor (Atom) caused the problem, I ran into the astyle issue and ran astyle veh_interact.cpp. Which made way more changes than I expected. However it seems that make astyle worked, so I'll do that in the future.

@anothersimulacrum
Copy link
Member

You need to run astyle with astyle --options=.astylerc for our formatting.

@Jerimee
Copy link
Contributor

Jerimee commented May 3, 2020

I understand the problem but not the solution.

I must be missing something because a number of folks have bumped this. Why are "skills required" and "tools required" red in the after but not the before?

Were they fulfilled in the before but not the after? If they are fulfilled in the after why are they red? Confused...

@procrustesmethod
Copy link
Contributor Author

Sorry for the inconsistency. I took the screenshots before and after I changed it. Before I added the changes, I spawned in a test character with a screwdriver and mechanics, so skills & tools were green. Afterwards, I forgot to change anything & left my character as-is. Then, to demonstrate color when all conditions unfulfilled, I debugged his strength to 0.

It's just inconsistency on my part, w.r.t the skills & tools I used to demonstrate.

The only change in code was under the Additional Requirements section of the interface, so the only meaningful distinction in the screenshot is in that one line of text below additional requirements.

@procrustesmethod
Copy link
Contributor Author

To be more specific about the solution: look at lines 1758 and 1759.
I make aid_color represent color for text "LIFTING X", and str_color represent color for text "STRENGTH X" (or "STRENGTH (ASSISTED) X).

For aid_color: if a sufficient tool for LIFTING is present, its color is set to green. If not, it checks if your character (or character+npc) have sufficient strength. If strength requirement is met, its text is set to grey. If not, text is set to red. Similarly for str_color. This is the same convention used for crafting and installing. by having the text be greyed out, the game lets you know "you lack this thing, but that's okay." red is saved for when both lifting and strength requirements are not met, i.e. "things are not okay."

@mlangsdorf mlangsdorf added the Vehicles Vehicles, parts, mechanics & interactions label May 4, 2020
@kevingranade kevingranade merged commit 7161a41 into CleverRaven:master May 5, 2020
@procrustesmethod procrustesmethod deleted the veh-remove-colors branch May 5, 2020 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants