Rework repair menu

[Night-Pryanik]

Summary

Interface "Rework repair menu"

Purpose of change

• Resurrect abandoned #50294 by @WelpThatWorked.
• Closes #50048.

Describe the solution

The work was mostly done by @WelpThatWorked. I just replaced all references to obsoleted player class with Character class and added missing translation stuff.

Describe alternatives you've considered

None.

Testing

Got sewing kit. Got some damaged clothing and repair components. Activated sewing kit, repaired clothing.

Additional context

[pjf]

A quick test shows the menu is rendering fine for me, but only after a delay of about five seconds on my 2.6GHz CPU.

[Night-Pryanik]

Hmm, strange. I've got absolutely no delay on my rig, though I have 3.6 GHz CPU.

[pjf]

Steps to reproduce:

1. Download RepairDelaySave.zip

2. activate the tailoring kit (n)

[Night-Pryanik]

Still no delay with your world on my rig.

https://user-images.githubusercontent.com/11132525/182448832-eded4a7f-80f9-465c-89de-a9814784d6eb.mp4

[descan]

Related to a suggestion I made yesterday, would it be possible to make multi-select work on this menu, in some form or fashion? The crux of my issue was being able to one-and-done repair/refit/reinforce multiple items, rather than re-hit the keys over and over, especially re-navigating the long list of items needing fixing up every time one is finished. Multi-select on this menu would solve that issue for my tastes, and close it.

[pjf]

Still no delay with your world on my rig.

Huh! That is very delay-free! I wonder why I'm special?

In any case, don't let me weird pause be a reason not to merge. This is so much nicer than what we had before. Thank you for resurrecting it!

[Fris0uman]

Since NPCs can't use the repair menu shouldn't we use avatar instead of Character?

[Night-Pryanik]

Since NPCs can't use the repair menu shouldn't we use avatar instead of Character?

I don't know, I just noticed that most inventory_presets in game_inventory.cpp uses Character despite these presets are clearly player-usable only: gun modification, cutting up items, sawing stock and barrel, attaching MOLLE, and many others.

[EIIKaO]

A quick test shows the menu is rendering fine for me, but only after a delay of about five seconds on my 2.6GHz CPU.

I got a same problem on my 3.7GHz CPU. Standing on an empty road(but wearing and carrying a lot) : 6-8 sec. Sitting at a desk where full of trash and materials : over 30 sec.

And after analyzing the problem I think I found the bottleneck(or you guys may have already caught the cause of the problem …).

repair_chance function(Specifically, in find_repair_difficulty called by repair_chance) is that each time called compares the item to be repaired with all recipes? in the recipe dictionary(recipes about 5500 currently).

See below code.

static int find_repair_difficulty( const Character &pl, const itype_id &id, bool training )
{
    // If the recipe is not found, this will remain unchanged
    int min = -1;
    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 );
    }

    return min;
}

So, I designed one solution to [minimum] modify the code. My idea is here.

static int find_repair_difficulty( const Character &pl, const itype &it, bool training )
{
    // If the recipe is not found, this will remain unchanged
    int min = -1;
    const int def_diff = 5;
    recipe uncraft_recipe = recipe_dictionary::get_uncraft( it.get_id() );
    
    if( it.recipes.size() == 0 && uncraft_recipe ){
        int uncraft_difficulty = uncraft_recipe.difficulty;
        if( uncraft_difficulty == 0 ) {
            uncraft_difficulty = -1;
        } else if( !training && !pl.has_recipe_requirements( uncraft_recipe ) ) {
            uncraft_difficulty++;
        }
        min = std::min( uncraft_difficulty, def_diff );
    } else {
        for( const recipe_id &rid : it.recipes ) {
            const recipe &r = recipe_id( rid ).obj();
            if( !r ) {
                continue;
            }
            // If this is the first time we found a recipe
            if( min == -1 ) {
                min = def_diff;
            }

            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 );
        }
    }
    return min;
}

and

int repair_item_actor::repair_recipe_difficulty( const Character &pl,
        const item &fix, bool training ) const
{
    int diff = find_repair_difficulty( pl, *fix.type , training );

    // If we don't find a recipe, see if there's a repairs_like that has a recipe
    if( diff == -1 && !fix.type->repairs_like.is_empty() ) {
        diff = find_repair_difficulty( pl, fix.type->repairs_like.obj() , training );
    }

    // If we still don't find a recipe, difficulty is 10
    if( diff == -1 ) {
        diff = 10;
    }

    return diff;
}

find_repair_difficulty is only called by repair_recipe_difficulty, and repair_recipe_difficulty is called from only two line in iuse_actor.cpp.

Tested repair some items in game, it works well(delay might be 0.1 second or less on the road, 0.5 sec or less on the terrible desk). Of course, my code is messy[no doubt], may be wrong and could be unsafe. Just for your information (if it is worth referring to).

Notes: There were some differences in success[damage] chance between the original code and my code. As far as I monitored in the debug log, seems to the "ballistic_vest_esapi" is having a difficulty value of 1 (0 before compensation) in the original code. 3 is my code value.

On original code:

On my code:

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

[Wishbringer]

The performance is horrible, as stated above, at least for me on i686-w64-mingw32.

[Fris0uman]

Open a bug report please