skyrim-multiplayer / skymp

Multiplayer Mod & TypeScript SDK for Skyrim Special Edition
Other
221 stars 75 forks source link

[SP Bug]: containerChanged event is triggered an uncountable number of times when leveling up #1314

Open Vorganger opened 1 year ago

Vorganger commented 1 year ago

Severity

Major. A crucial error that indicates a deviation from business logic or disrupts the program, but doesn't have a vital impact on the app.

Priority

Medium. Anything that negatively affects the user experience.

Description

I noticed that the containerChange event is triggered an uncountable number of times when leveling up.. I noticed this when I ran a printConsole("Hello"), and found out that the console was filled with "Hello". Scrolling up seems to not show whatever came before leveling up. Also, since this event is triggered a lot, there is a minor stutter when leveling up. This is especially noticeable if there is more code (more operations) in that event

I worked around this by returning if the "LevelUp Menu" is opened, but that of course doesn't really help the issue itself. This will still run that check an uncountable number of times and still have a minor stutter

OS

Windows

OS version

10

Skyrim version

1.5.97

SKSE version

2.0.20

client commit''s hash

None

server commit''s hash

None

Videocard model

RTX 2060

Steps to reproduce

Have a Skyrim Platform plugin that has some code in a "containerChange" with printConsole("Something") and level up your character in the level up menu. This will run the containerChange event an uncountable number of times

Expected result

Container change does not run at all when leveling up

Actual result

Container change runs an uncountable number of times and produces a stutter (varying from user to user)

Pospelove commented 1 year ago

@Vorganger Thanks for reporting this What arguments does containerChange have in case of levelup? e.oldContainer, etc?

Vorganger commented 1 year ago

Thanks for the reply

I used the containerChanged event for updating certain values. The only arguments contained in containerChanged is event.baseObj. Feel free to let me know if you need more information to work on this issue

Vorganger commented 1 year ago

I checked event.oldContainer and event.newContainer and found out they change all (or a majority of) the NPCs in the game, which makes sense since Skyrim has level scaling. After noticing that, I added some code that would either return if the level up menu is open or if the containers do not involve the player. So I guess this issue is a non-issue if the following code is used. I should note that isLevelUpMenuOpen is only changed when the menuOpen (true) and menuClose (false) events are triggered, which is more optimal than using the Ui.isMenuOpen() function. Also, consts.PLAYER_ID is declared elsewhere and is 0x14.

on("containerChanged", (event) => {
    if (isLevelUpMenuOpen) {
        return;
    }
    let oldContainerId: number = event.oldContainer ? event.oldContainer.getFormID() : 0;
    let newContainerId: number = event.newContainer ? event.newContainer.getFormID() : 0;
    if (oldContainerId !== consts.PLAYER_ID && newContainerId !== consts.PLAYER_ID) {
        return;
    }
    ...
});

Edit: Changed the code part to be in "code" format and added clarifications on variables declared elsewhere.

ZikkeyLS commented 1 year ago

@Pospelove it should possibly close problem for now, what would you say?