new function in pmdm

[Kinak338]

I looked into some functions in pmdm, I'm not sure if I have done everything correctly, feel free to make any suggestion

---

This change is 

[Pistonight]

Reviewable status: 0 of 3 files reviewed, 14 unresolved discussions

---

src/Game/UI/uiPauseMenuDataMgr.h line 12 at r1 (raw file):

#include <math/seadVector.h>
#include <prim/seadSafeString.h>
#include <prim/seadScopedLock.h>

Looks like this is not needed?

---

src/Game/UI/uiPauseMenuDataMgr.h line 59 at r1 (raw file):

// Going beyond this limit will glitch the menu.
constexpr int NumTabMax = 50;
constexpr int SizeTabMax = 20;

Maybe NumItemsPerTabMax is clearer

---

src/Game/UI/uiPauseMenuDataMgr.h line 400 at r1 (raw file):

    void grabbedItemStuff(PouchItem* item);
    PouchCategory getTabCategory(int tab_index);
    const sead::SafeString* getNameFromTabSlot(int tab_index, int slot_index);

Suggestion:

    PouchCategory getTabCategory(int tab_index) const;
    const sead::SafeString* getNameFromTabSlot(int tab_index, int slot_index) const;

---

src/Game/UI/uiPauseMenuDataMgr.h line 403 at r1 (raw file):

    PouchItem* getPouchItemFromTabSlot(int tab_index, int slot_index);
    bool isInInventoryFromTabSlot(int tab_index, int slot_index);
    bool isEquippedFromTabSlot(int tab_index, int slot_index);

Suggestion:

    bool isInInventoryFromTabSlot(int tab_index, int slot_index) const;
    bool isEquippedFromTabSlot(int tab_index, int slot_index) const;

---

src/Game/UI/uiPauseMenuDataMgr.h line 521 at r1 (raw file):

    void sellItem(PouchItem* target_item, int quantity);
    int getWeaponsForDpad(sead::SafeArray<PouchItem*, 20>* result_array, PouchItemType target_type);
    int getNumberOfItemsInTab(int tab_index);

Looks like these could be const.

Also shouldn't these be public?

Suggestion:

    int getWeaponsForDpad(sead::SafeArray<PouchItem*, 20>* result_array, PouchItemType target_type) const;
    int getNumberOfItemsInTab(int tab_index) const;

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 25 at r1 (raw file):

#include "KingSystem/GameData/gdtSpecialFlags.h"
#include "KingSystem/GameData/gdtTriggerParam.h"
#include "KingSystem/Resource/GeneralParamList/resGParamListObjectGlobal.h"

These header changes don't seem needed either

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 1256 at r1 (raw file):

    case PouchItemType::Bow:
    case PouchItemType::Arrow:
    case PouchItemType::Shield:

is this needed for match?

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3085 at r1 (raw file):

    for (int i = 0; i < slot_index; i++) {
        item = getItems().next(item);
    }

This seems uninline-able

Suggestion:

    const auto lock = sead::makeScopedLock(mCritSection);
    auto* item = getPouchItemFromTabSlot(tab_index, slot_index);

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3093 at r1 (raw file):

            bool found = false;
            for (int i = 0; i < mGrabbedItems.size(); ++i) {
                auto& item2 = mGrabbedItems[i].item;

Taking reference of a pointer is suspicious - can you make a decompme for this?

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3142 at r1 (raw file):

        break;
    }
    return;

remove this return

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3152 at r1 (raw file):

    for (int i = 0; i < 20; i++) {
        (*result_array)[i] = nullptr;
    }

Try

Suggestion:

    result_array->fill(nullptr);

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3160 at r1 (raw file):

    int i = 0;
    auto* item = list.nth(0);
    if (target_type != PouchItemType::Arrow) {

Looks a bit weird that the loop is duplicated based on this condition. Try to put the condition inside the loop. This might be a compiler optimization

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3161 at r1 (raw file):

    auto* item = list.nth(0);
    if (target_type != PouchItemType::Arrow) {
        for (; item && item->isWeapon() && i < 20; item = list.next(item)) {

Let's use the constant for 20, same below

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3251 at r1 (raw file):

    for (int i = 0; i < slot_index; i++) {
        item = nextItem(item);
    }

Try uninline this? Same below

Suggestion:

    auto* item = getPouchItemFromTabSlot(tab_index, slot_index);

[Kinak338]

Reviewable status: 0 of 3 files reviewed, 14 unresolved discussions (waiting on @Pistonight)

---

src/Game/UI/uiPauseMenuDataMgr.h line 12 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Looks like this is not needed?

Done.

---

src/Game/UI/uiPauseMenuDataMgr.h line 59 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Maybe NumItemsPerTabMax is clearer

Done.

---

src/Game/UI/uiPauseMenuDataMgr.h line 521 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Looks like these could be const.

Also shouldn't these be public?

They should be public, except getNumberOfItemsInTab which gets inlined in a bunch of place but isn't actually a function

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 25 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

These header changes don't seem needed either

Done.

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 1256 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

is this needed for match?

Done.

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3085 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

This seems uninline-able

If I do it this way, it will not return without calling updateInventoryInfo etc

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3093 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Taking reference of a pointer is suspicious - can you make a decompme for this?

Sure it is not matching because of something around here, I haven't found what I should write with those strings https://decomp.me/scratch/7ona5

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3142 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

remove this return

Done.

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3152 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Try

I tried that but it doesn't match because of some minor re-ordering

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3160 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Looks a bit weird that the loop is duplicated based on this condition. Try to put the condition inside the loop. This might be a compiler optimization

Maybe but that looks more difficult to get to match

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3161 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Let's use the constant for 20, same below

Done.

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3251 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Try uninline this? Same below

It doesn't uninline, getNumberOfItemsTab is already getting a common part of all of those functions that is getting inlined, maybe it is slightly bigger to include this part, but I don't see how to make return cases works

---

src/Game/UI/uiPauseMenuDataMgr.h line 400 at r1 (raw file):

    void grabbedItemStuff(PouchItem* item);
    PouchCategory getTabCategory(int tab_index);
    const sead::SafeString* getNameFromTabSlot(int tab_index, int slot_index);

Done.

---

src/Game/UI/uiPauseMenuDataMgr.h line 403 at r1 (raw file):

    PouchItem* getPouchItemFromTabSlot(int tab_index, int slot_index);
    bool isInInventoryFromTabSlot(int tab_index, int slot_index);
    bool isEquippedFromTabSlot(int tab_index, int slot_index);

Done.

[Pistonight]

Reviewable status: 0 of 3 files reviewed, 11 unresolved discussions (waiting on @Kinak338)

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3251 at r1 (raw file):

Previously, Kinak338 wrote…

It doesn't uninline, getNumberOfItemsTab is already getting a common part of all of those functions that is getting inlined, maybe it is slightly bigger to include this part, but I don't see how to make return cases works

Maybe getNumberOfItemsInTabs isn't its own function and is part of getPouchItemFromTabSlot, which is then inlined into the other functions?

---

src/Game/UI/uiPauseMenuDataMgr.h line 244 at r2 (raw file):

            mData.weapon.mModifierValue = value;
    }
    void setWeaponModifierInfo(const act::WeaponModifierInfo* info);

Do we need this?

[Pistonight]

Reviewable status: 0 of 3 files reviewed, 11 unresolved discussions (waiting on @Kinak338)

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3152 at r1 (raw file):

Previously, Kinak338 wrote…

I tried that but it doesn't match because of some minor re-ordering

Try with the loop optimization below?

[Kinak338]

Reviewable status: 0 of 3 files reviewed, 9 unresolved discussions (waiting on @Pistonight)

---

src/Game/UI/uiPauseMenuDataMgr.h line 244 at r2 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Do we need this?

I don't know where that comes from

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3251 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Maybe getNumberOfItemsInTabs isn't its own function and is part of getPouchItemFromTabSlot, which is then inlined into the other functions?

It doesn't want to inline getPouchItemFromTabSlot, I don't know what I should do to force it

[Kinak338]

src/Game/UI/uiPauseMenuDataMgr.cpp line 3152 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Try with the loop optimization below?

It does look better but it doesn't really match https://decomp.me/scratch/tMOum

[Pistonight]

Reviewable status: 0 of 3 files reviewed, 8 unresolved discussions (waiting on @Kinak338)

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3152 at r1 (raw file):

Previously, Kinak338 wrote…

It does look better but it doesn't really match https://decomp.me/scratch/tMOum

Yeah I couldn't get it to match either, i guess this version is fine, can we replace the 20 with the constant again? including the type parameter

[Pistonight]

Reviewable status: 0 of 3 files reviewed, 8 unresolved discussions (waiting on @Kinak338)

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3251 at r1 (raw file):

Previously, Kinak338 wrote…

It doesn't want to inline getPouchItemFromTabSlot, I don't know what I should do to force it

Try define getPouchItemFromTabSlot in the header?

[leoetlino]

Reviewed 1 of 3 files at r2, all commit messages.
Reviewable status: 1 of 3 files reviewed, 8 unresolved discussions (waiting on @Kinak338 and @Pistonight)

---

src/Game/UI/uiPauseMenuDataMgr.h line 400 at r1 (raw file):

Previously, Kinak338 wrote…

Done.

you missed getTabCategory

---

src/Game/UI/uiPauseMenuDataMgr.h line 244 at r2 (raw file):

Previously, Kinak338 wrote…

I don't know where that comes from

if this isn't used, we should remove this

---

src/Game/UI/uiPauseMenuDataMgr.cpp line 3093 at r1 (raw file):

Previously, Kinak338 wrote…

Sure it is not matching because of something around here, I haven't found what I should write with those strings https://decomp.me/scratch/7ona5

https://decomp.me/scratch/pKLf7