ChaosMarc/PlugY

Unassign Skill Points button crashes every time

Closed this issue · 18 comments

Windows 10
Diablo 2 LoD 1.14d
PlugY 12.00

I've only tried it three times, but every time I press the button to unassign all of my skill points, the game crashes. Playing a Sorc in A1 Normal right now if that matters.

[SKILLS POINTS]
ActiveSkillsUnassignment=1
ActiveSkillsUnassignmentOneByOne=0

this branch has a bug, you can use this:
https://github.com/haxifix/PlugY

Are there any compatibility issues to be aware of when switching between implementations of PlugY?

There shouldn't be any complications. Also I will look into this. Must be a merge conflict.

I have tested this and do not have any problems. Did you create a new character or did this character come from a lower Diablo version?

I can reproduce it with 1.14d (all other versions still work fine). the problem does not occur with the pre-build dll that is available in haxifix' repository. but if I build it from his sources the bug occurs too. I feel the creeping dread that he forgot to check in some changes

I have forked your sources and updated mine to yours when you applied halifax's code. When I build it. I still do not see that problem.

could you upload the plugy.dll you built?

edit: i compiled your sources and got the same problem. looks like it's a Visual Studio 2019 problem. Would you be interested in working together on this? if you want we can talk via discord (ChaosMarc#8858) or telegram (https://telegram.me/chaosmarc)
image

You could get it from the installer I created here. https://github.com/markm11/PlugY/blob/master/PlugYInstaller/PlugY_The_Survival_Kit_v12.00.exe
Just know this has changes to where the plugy.ini lives. It has to reside in where the plugy.dll lives. This was a fix I put in for D2Modmaker. I am using Visual Studio 2017.

I will try compiling it with VS2017.
edit: even if I compile your repository's code with VS2017 the problem still occurs
edit2: I tried the installer you linked. The game still crashes. So it's not the way it's compiled but depends on the runtime environment. Which version of Windows are you using? and to be absolutely sure: You're testing with patch 1.14d, right?

I like the changes in your repository (based on the commit messages). perhaps you could create a PR with some documentation of what was changed exactly?

Windows 10 as well. I will try the crashing builds on win 8.1 tomorrow.
For now I tried to debug the problem. the crash occurs in SkillsPoints.cpp:71. ptSkill->ptNextSkill is NULL for the last skill in the loop. if I choose ignore the ingame error (with a debug build, or while debugging) the unassignment process is successful. But I have no clue what this means :D seems fine to me, as the last skill in a list wouldn't have a nextSkill ptr. but the code worked before, so it shouldn't be a problem...

tested it. for me the game crashes on win8.1 enterprise as well

@ChaosMarc Maybe your compiler setting has some problem, I'm not sure. For example, you set Enable Minimal Rebuild to Yes, you should recompile all files again.

I feel embarrassed. For some reason I was thinking stats not skills. Once I saw where you were in the code I realized my mistake.
I can reproduce this bug. That part of the code causing the problem is a call to this function "D2AddPlayerStat( ptChar, STATS_NEWSKILLS, nbPoints, 0 );" Line 72 in SkillPoints.cpp. I will keep looking into this. Forgive me for my stupidly.

After looking into it more I believe that problems is with this code.
FCT_ASM(D2SetSkillBaseLevelOnClient_114)
PUSH EBX
PUSH EBX
PUSH ESI
PUSH DWORD PTR SS : [ESP + 0x14]
PUSH DWORD PTR SS : [ESP + 0x14]
MOV EBX, DWORD PTR SS : [ESP + 0x14]
MOV EAX, ECX
MOV ESI, EDX
CALL V2SetSkillBaseLevelOnClient
POP ESI
POP EBX
RETN 0xC
}}

It is changing the pointer value of "ptChar". Only in the Release build.

The only place this is getting called is in SkillPoints.cpp.

@ChaosMarc when debugging the code you can actually turn off the debug error so you can continue with debugging.

@markm11 that's really great news. Don't worry about the misunderstanding. I'm just happy that you found the bug for yourself :)

yeah I found out that you could just debug over it too. the respec itself works fine. the error message says something about "the value of esp was not properly saved across a function call". google let me to this answer which seems helpful (for someone knowing what he does ^^): https://stackoverflow.com/a/1332874

If you haven't seen it: @haxifix must have had the same problem as he created this commit: haxifix/PlugY@5854956 "Fix skill respec". But I have the feeling that he just forgot to commit one of his changes.

I looked at his code and it is already in my code. I will keep looking at it and I think I have fixed it. I have to leave and go to my job and will let you know later how it works. My release code is now not changing the pointer.
[10800] Value of curChar 117057792
[10800] Value of curChar 117057792
[10800] Value of curChar 117057792
[10800] Value of curChar 117057792
[10800] Value of curChar 117057792
[10800] Value of ptChar 117057792

fixed by #30
many thanks to you @markm11