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 menu shows slow #61438

Merged
merged 5 commits into from Oct 9, 2022
Merged

Conversation

EIIKaO
Copy link
Contributor

@EIIKaO EIIKaO commented Oct 3, 2022

Summary

Performance "Fix Repair menu shows slow"

Purpose of change

  • Fix Repairing articles of clothing is laggy asf #61380
    • Stopped comparing against all recipes every time one item's repair probability is calculated, and instead used the recipes held by the item type.
  • Now use the difficulty in uncraft recipes.
    • And if the recipe did not define the difficulty, 5( on items' recipe and obsolete recipes ) or 0 ( on uncraft recipe ) are used for the default. As a result, some items show changes in repair success chances( see details in Testing and Additional context ).

Describe the solution

repair_recipe_difficulty function is that each time called compares the item to be repaired with all recipes in the recipe dictionary( recipes over 5,500 currently ).

If about 50 items appear in the repair menu, then at least 50*5,500( 275,000 ) comparisons will be done. I have concluded that the large number of times this comparison occurs is the bottleneck of the problem.

So I stopped comparing with all recipes and used the recipes held by the item type. If a recipe is not held or matched, it is matched either with the dictionary of uncraft or obsoletes. If none is found, -1 is returned.

Describe alternatives you've considered

Use a cache for comparison with all recipes ( converted to vector, not map, and kept in recipe_dictionary, but did not improve performance enough as far as I tried ).

Testing

In-game testing done. Performance was improved dramatically( in one case, what used to take about 30 seconds became 0.35 seconds ), and the returned difficulty level was as designed.

As far as I could confirm in-game, there were changes in the repair success chance. This is probably all due to the difference in the handling of difficulty 0, which is explained later.

  • Decrease cases
    turnout coat ( current code returned 0 because the obsolete recipes were not defined difficulty )
    US ballistic vest ( cur code returned 0 because it has obsolete recipe and the other recipe in uncraft recipes with difficulty 2 was ignored )
  • Increase cases
    acetylene torch, processor board, oxygen tank, flare, e-ink tablet PC ( current code does not search for uncraft recipes, determining no recipe and returning difficulty -1 )
    some of materials ( i.e. sheet of kevlar, lycra, synthetic fabric and etc... )

Additional context

Difficulty 0 has two main types.

The first one is explicitly defined as 0.

  1. This meaning is very clear. That is, the difficulty level is very low( however, there does not seem to be a recipe with 0 defined in vanilla ).

The second one is complicated.

  1. The most common cases would be that there is no need to define it, in other words, it is not expected to be created( but we can repair it ).
  2. In other cases, by not defining it, they may expect a default value to be assigned.
  3. Moreover, there may be cases where they forgot to define it.

In this PR, I decided to use 5 if the difficulty level was not defined. It is expected that case (2.) will be largely affected.

Advice and criticism welcome.

Notes:

*Updated
It seems that there are cases where delays do not occur, evidenced by the video presented by @Night-Pryanik #59827 (comment).
In my environment there is about 6.5 seconds delay, almost the same as @pjf who provided saved data for testing.
If someone has an environment without delays, I would appreciate it if you could provide me with information ( PC environment, OS, etc. ) or your opinion.

*Updated 2
While reading the latest release code, I found a strange point.

for( const auto &e : recipe_dict ) {
const recipe r = e.second;
if( id != r.result() ) {
continue;
}
// If this is the first time we found a recipe
if( min == -1 ) {
min = 5;
}
int cur_difficulty = r.difficulty;
if( !training && !pl.knows_recipe( &r ) ) {
cur_difficulty++;
}
if( !training && !pl.has_recipe_requirements( r ) ) {
cur_difficulty++;
}
min = std::min( cur_difficulty, min );
}

In this case, the difficulty would return a maximum of 5 ( 7 after adjustment +2). As a result, a handheld game system with a difficulty of 8 in the recipe, for example, will return a minimum of 5 and a maximum of 7, making it easier to repair than expected. I wonder if this is as designed.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) Code: Performance Performance boosting code (CPU, memory, etc.) astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 3, 2022
Optimize code for Clang-tidy (clang-12, tiles) / build (pull_request)
@github-actions github-actions bot removed the astyled astyled PR, label is assigned by github actions label Oct 4, 2022
Astyle
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels Oct 4, 2022
Forgot to attach the updated file
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 4, 2022
@EIIKaO EIIKaO marked this pull request as draft October 5, 2022 18:25
Revised logic of difficulty calc
@EIIKaO EIIKaO marked this pull request as ready for review October 5, 2022 20:20
@KymasTuran
Copy link
Contributor

Hope this is merged soon. I m repairing naked and with just the broken item. (old pc)
Are there plans to change the entire repair system?

@dseguin dseguin merged commit d3894e4 into CleverRaven:master Oct 9, 2022
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 <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` Code: Performance Performance boosting code (CPU, memory, etc.) json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repairing articles of clothing is laggy asf
3 participants