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

Fix repair ui getting stuck on composite plating/rams #60139

Merged
merged 1 commit into from
Aug 15, 2022

Conversation

irwiss
Copy link
Contributor

@irwiss irwiss commented Aug 14, 2022

Summary

None

Purpose of change

Recent change on military composite plating put NO_REPAIR flag, which is confusing vehicle code, this fixes it so it doesn't

Describe the solution

Removes "repair" key from "requirements" so vehicle UI isn't getting stuck on repairing composite

Describe alternatives you've considered

Removing the misleading flag is apparently invalid

Testing

Spawn lightly damaged humvee, see r brings you to repair plating but shows repairing disabled message similar to attached screenshot.
Apply patch, do the above, you should not be able to select the plating at all (or any other non-repairable part) when in 'r' mode.

Additional context

Untitled

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Code: Tests Measurement, self-control, statistics, balancing. Items: Battery / UPS Electric power management Mods Issues related to mods or modding Mods: Aftershock Anything to do with the Aftershock mod Vehicles Vehicles, parts, mechanics & interactions json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Aug 14, 2022
@irwiss irwiss marked this pull request as ready for review August 14, 2022 07:17
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Aug 14, 2022
@anothersimulacrum
Copy link
Member

I think the flag may be useful to have as a flag to give an error during load if a part with it has repair info.

Regardless of that, I don't think the test is a useful test to have.

@irwiss
Copy link
Contributor Author

irwiss commented Aug 14, 2022

The test is the error though, even before a player is loading the game; if someone wants to specify vpart can't be repaired and goes for the 'naive'/obvious solution of reusing item flag instead of deleting 'repair' key - the PR will fail tests and PR author will get the correct solution displayed as test failure message

@anothersimulacrum
Copy link
Member

anothersimulacrum commented Aug 14, 2022

The test is not useful because nobody is going to be adding the flag if it's not used anywhere and not mentioned in the documentation (which it should be, I assume that not happening here is an unintentional omission). This kind of thing can be useful, but it needs to occur during JSON validation/loading, not in a test (and generally not for a single specific flag), because the vast majority of people editing JSON are not running the tests, and are especially not doing it immediately after adding something, so it essentially only provides feedback to people who haven't tested their PR before pushing it. We do this sort of flag validation with items (and I believe monsters).

Having the flag remain as a method to opt in to error reporting in the case that for some reason a part has repair defs it should not is useful, because "I am able to repair a part I shouldn't be able to" is not an issue that is liable to be reported or even known is an issue.

@irwiss irwiss force-pushed the fix-vpart-no-repair-confusion branch from 7d701ae to 3c34bab Compare August 15, 2022 10:12
@irwiss irwiss changed the title Fix vpart NO_REPAIR confusion Fix repairing getting stuck on composite plating/rams Aug 15, 2022
@irwiss irwiss changed the title Fix repairing getting stuck on composite plating/rams Fix repair ui getting stuck on composite plating/rams Aug 15, 2022
@irwiss
Copy link
Contributor Author

irwiss commented Aug 15, 2022

Well this goes beyond the scope of my caring at this point, so i adjusted this to just fix the issue that bothered me - the vehicle repair ui no longer gets stuck on composite stuff the player can't repair

@dseguin dseguin merged commit 1bc5885 into CleverRaven:master Aug 15, 2022
@irwiss irwiss deleted the fix-vpart-no-repair-confusion branch August 16, 2022 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. Items: Battery / UPS Electric power management [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Mods: Aftershock Anything to do with the Aftershock mod Mods Issues related to mods or modding Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants