Jump to content

Photo

Lord Khelben's Bugs and Questions Thread


39 replies to this topic

#1 khelban12

khelban12
  • Members
  • 121 posts

Posted 03 August 2016 - 12:22 AM

Greetings once more.

I thought instead of flooding the forum by opening a new thread every now and then, to make a thread that has all my questions here.

I had a surgery and didn't have much time to play bg2 but now that i found some time, i have more questions :)

1) Has anyone noticed weird behavior with container bags and shops ?

When i click on the bag of holding while in my inventory, i can open it fine and see what items it contains. However, when am in the "shop screen", i cannot open it so i can sell the things it contains. I have a quite modded installation but i removed the mods and tried without them and i get the same behavior. Also i tried older gemrb released up to 0.8.0 i think and i still cannot open the bag. This only happens when i am in a shop.

Does anyone else have this behavior or i have messed something up in my case ?

2) HP calculation of PC/NPCs.

Some months ago, i noticed that my character had very few HP. I finished the whole Irenicus Dungeon and didn't notice it at first but when i loaded a save game in the vanilla engine all the characters had way more HPs.

So again, i tried to find what function is responsible for this and tracked it to the following code:
 
hptm.load(tm->QueryField(classname, "HP"), true);
if (hptm) {
	int tmphp = 0;
	int rollscolumn = hptm->GetColumnIndex("ROLLS");
	while (atoi(hptm->QueryField(tmphp, rollscolumn)))
		tmphp++;
	buffer.appendFormatted("HPROLLMAXLVL: %d", tmphp);
	if (tmphp) maxLevelForHpRoll[tmpindex] = tmphp;
}
So, it queries classes.2da for the proper "HP" table (hprog, hpwiz, etc), opens it and while the ROLLS column does not have a 0 value, it increments the tmphp variable. So the character gets bonus HP for every level that he rolls dice. This works fine.

There is a BG2 Tweaks component though that changes the HP calculation to give creatures maximum HP and does it by multiplying SIDES * ROLLS and putting that to the modifier and then putting 0 to ROLLS. So, instead of getting 2*3+0 that is 1D6, you are getting 2*0+6.

GemRB has a "Maximum HP" setting ofcourse but i installed the component so that it wms in the vanilla engine. All this time, i had modified gemrb to use the "MODIFIER" column instead of the "ROLLS" column and check for when the modifier changed and this worked fine. However, the Tweaks Anthology (which replaces BG2 Tweaks) has an extra component which can give Maximum HP only in Level 1. This breaks both the original GemRB code and mine.

So back to the drawing board. I don't know if it is the proper solution but the only one i could think of, is no to use such a "calculating" code but to use the "hardcoded" level values. Mages/Thieves get Constitution bonus up until L10 and everyone else up until L9.

I inserted another column to bg2/classes.2da and modified the code to read the level from there. I didn't tested it thoroughly but with minimal testing in BG2, it works fine and i get the same HP as before. I guess classes.2da from other games must be modified in the same way.
Spoiler


What are your thoughts about this ? Is this acceptable code ? Is there a better solution than this ? It isn't worth it to change at all since GemRB has its own Maximum Hp Setting ?

Edit:
3) NPC Level

NPCs have many creature files with different levels. When you meet one, you get the appropriate version for your level. This is also the case for Jaheira and Minsc in the Starting Dungeon.

When you create a rogue PC, the starting XP of 89000 results in a L8 character. The vanilla engine gives you a L8-L7 Jaheira with 81000*2 XP and a L8 Minsc with 161000 XP. However, GemRB gives you the lower creature files that you get with a normal L7 PC.

I tracked it down to the CheckForReplacementActor in Game.cpp. The code in question was changed by the following commit:

commit 66124ed39297424cf7561f2fe249a52f78b66e80
Author:     Jaka Kranjc <lynxlupodian@users.sourceforge.net>
AuthorDate: Mon Apr 23 19:33:59 2012 +0200
Commit:     Jaka Kranjc <lynxlupodian@users.sourceforge.net>
CommitDate: Mon Apr 23 19:33:59 2012 +0200

    the npclevel 2da does not contain replacement for high levels

diff --git a/gemrb/core/Game.cpp b/gemrb/core/Game.cpp
index 018b7a2..80c6321 100644
--- a/gemrb/core/Game.cpp
+++ b/gemrb/core/Game.cpp
@@ -876,7 +876,12 @@ bool Game::CheckForReplacementActor(int i) {
                std::vector<std::vector<char *> >::iterator it;
                for (it = npclevels.begin(); it != npclevels.end(); it++) {
                        if (!stricmp((*it)[0], act->GetScriptName()) && (level > 2)) {
-                               strncpy(newcre, (*it)[level-2], sizeof(ieResRef));
+                               // the tables have entries only up to level 24
+                               ieDword safeLevel = npclevels[0].size() - 1;
+                               if (level < safeLevel) {
+                                       safeLevel = level;
+                               }
+                               strncpy(newcre, (*it)[safeLevel-2], sizeof(ieResRef));
                                break;
                        }
                }
Should that -1 be there when safeLevel is already decremented by 2 ?

diff --git a/gemrb/core/Game.cpp b/gemrb/core/Game.cpp
index a333e9b..bd2fd42 100644
--- a/gemrb/core/Game.cpp
+++ b/gemrb/core/Game.cpp
@@ -972,7 +972,7 @@ bool Game::CheckForReplacementActor(int i)
                for (it = npclevels.begin(); it != npclevels.end(); it++) {
                        if (!stricmp((*it)[0], act->GetScriptName()) && (level > 2)) {
                                // the tables have entries only up to level 24
-                               ieDword safeLevel = npclevels[0].size() - 1;
+                               ieDword safeLevel = npclevels[0].size();
                                if (level < safeLevel) {
                                        safeLevel = level;
                                }
I removed the decrement and now i get the proper NPCs in all cases but as always i only tested it minimally and only in BG2.

Thank you for your time.

Edited by khelban12, 03 August 2016 - 01:01 AM.


#2 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 03 August 2016 - 02:32 AM

It's actually better to have separate threads, so the info is easier to search for and linked to.

 

1. I'm pretty sure we just don't have support for nested shops yet (a single CurrentStore marker). Inventory bags are technically stores too.

2. Yeah, it looks good. The values for other games would be the same, only perhaps pst needs different (iwd2 doesn't use this). Our extend2da.py doesn't know about insertion at col index or rearrangement, but maybe NI does (only checked DLTCEP).

The annoying thing is that we can't get rid of maxLevelForHpRoll and just use the table values for all, since it also dictates for how long the constitution bonus to hp should be applied (which you noticed too).


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...


#3 khelban12

khelban12
  • Members
  • 121 posts

Posted 03 August 2016 - 05:49 AM

It's actually better to have separate threads, so the info is easier to search for and linked to.

Ok, i will use separate threads then.
 

1. I'm pretty sure we just don't have support for nested shops yet (a single CurrentStore marker). Inventory bags are technically stores too.

That is why it didn't work then :) Thank you for clarifying it to me. I didn't know that.

2. Yeah, it looks good. The values for other games would be the same, only perhaps pst needs different (iwd2 doesn't use this). Our extend2da.py doesn't know about insertion at col index or rearrangement, but maybe NI does (only checked DLTCEP).

Nice, i didn't know about extend2da.py. I did it from vim's block mode where you can easily delete/insert columns in cases like this.

Do you have any thoughts about the -1 decrement in 3 ?

Thanks again.

#4 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 03 August 2016 - 06:20 AM

I didn't see #3 before. It looks like you're correct. The -1 is there due to the first element being the row name, but since the columns start at two, it should also have a +1, effectively zero. Nice catch!


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...


#5 khelban12

khelban12
  • Members
  • 121 posts

Posted 17 August 2016 - 04:13 AM

I compiled the latest (0163d5ed commit) gemrb to start a new playthrough and i thought to mention some modifications i do locally in case anyone likes them (besides those that i have already mentioned in older posts).

1) Fix for Reform party window
reform-window.jpg

Regardless of how many NPCs are in the party, the reform window always shows an extra border at the far right of the screen. I tracked it down to the function GetPortraitButtonPairs which is correctly called with ExtraSlots=1, but contains the code min(oldSlotCount, MAX_PARTY_SIZE) = min(6 + 1, 6) so the ExtraSlots being 1 is not taken into account. I changed the code from min() to max() and it works but it looks wrong to me to change the logic of the code. Maybe some other steps need to be taken.

Works fine in my case but not recommended.
Spoiler


2) Character Creation screen shows the total number of points rolled and also doesn't allow rolls less than 75 like the vanilla engine.
I used the label just left to the remaining points (it used to just say Abilities) to print the total number of points.

Spoiler


3) Modify the Trap finding code. Controversial change significantly modifying the Trap behavior but i think that it is correct.
GemRB divides that Find Traps skill by 2 and then makes a roll with that (rolls from 1 to FT/2). Then it adds this roll to the second half of the skill and if it is larger than the trap difficulty, then the trap is disabled. If not, then the trap is triggered.

I guess it does that to have an element of randomness in the trap process (i tracked the code back to 2003 but it doesn't document the decision).

However, i do not think the vanilla engine behaves in the same way. I noticed this difference the first time i used gemrb because i triggered some traps in the Irenicus Dungeon when i never had in the vanilla engine. I do not have any proof of this (maybe Avenger or someone else with access to the EE code can shed a light how traps work) but i think the vanilla engine behaves without rolling dice. When your Find Traps skill is larger than the trap difficulty, you disarm it every single time. Also, when you do not disarm it, it is not automatically triggered unless you move into it. I tested this in Ellesime's room in the Irenicus Dungeon. I had a character try to disarm a trap and fail over 50-60 times and it was never triggered (i remember in the past triggering some traps so maybe there is a 1% fail like in pickpocketing and hiding in shadows).

So i modified the code to a) not roll at all but use the whole skill without dividing it, b) not trigger the trap when fail to disarm.

Spoiler


4) Fix abilities order.
vanilla-thief.jpg
gemrb-thief.jpg

Thieves have the Hide in Shadows and Thieving abilities reversed. I changed qslots to mimic the vanilla engine.

Spoiler


#6 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 17 August 2016 - 10:21 AM

1. The check is there to prevent trying to access non-existent default controls. Does a  MAX_PARTY_SIZE + ExtraSlots work as expected?

 

2. Was this always true or is only true for lower difficulties?

 

3. I don't know if this originated from RE or from a choice, but considering the pickpocket RE didn't show anything like this, I'd wager you're correct. The triggering bit is more complicated. Could be different amongst games, among doors/containers/infopoints and potentially another thing affected by difficulty.

 

4. Ripe for a pull request. :)


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...


#7 khelban12

khelban12
  • Members
  • 121 posts

Posted 17 August 2016 - 11:39 AM

1. The check is there to prevent trying to access non-existent default controls. Does a  MAX_PARTY_SIZE + ExtraSlots work as expected?


for i in range(min(oldSlotCount, MAX_PARTY_SIZE + ExtraSlots))
Do you mean like the above ? If yes, then yes it works that way too since it doesn't cancel the ExtraSlots.
 

2. Was this always true or is only true for lower difficulties?

I don't know much about this but i always remember reading in fora that you couldn't get a score lower than 75.

3. I don't know if this originated from RE or from a choice, but considering the pickpocket RE didn't show anything like this, I'd wager you're correct. The triggering bit is more complicated. Could be different amongst games, among doors/containers/infopoints and potentially another thing affected by difficulty.

There were surely times i failed disarming a trap and didn't trigger it even on higher difficulty settings but until someone can verify that this is the way the engine works, then yes do not trust my word on this. I may remember wrong.

4. Ripe for a pull request. :)


Yeah, a pull request would make it easier for you and i will also not bother the forum, but i have stopped using github (nor i considered that my suggestions were good enough to include).

#8 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 17 August 2016 - 01:11 PM

1. Ok, did that.

2. Definitely something to test. With a human or something with low racial minimums and a similarly free class (vs. paladin's high requirements). I've added it to the wiki research list that nobody seems to look at. Maybe I should sticky it here.

3. Yep, just more testing required.

4. It's fine for few-liners, but how about uploading the patches to a pastebin next time? I could then apply them directly (most have a raw mode). But this one had no whacky spaces, so it applied in a here doc nicely. :)

 

Thanks and don't think so badly of your work!


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...


#9 Jarno Mikkola

Jarno Mikkola

    The Imp

  • Modders
  • 6829 posts
  • Gender:Male
  • Location:The town where the dead haven't keeled over, yet. In Finland.

Posted 17 August 2016 - 02:44 PM

2. Definitely something to test. With a human or something with low racial minimums and a similarly free class (vs. paladin's high requirements).

Nope. Fighter is far better, it's minimums are 9,3,3,3,3.... everyone else will get more or equal. Because the paladin's cha is minimum of 17, it's that at least, in the old goldbox games, you couldn't pick a class before you had the base stats, so you might not have been able to be a paladin because you lacked the points, but in BG's you choose the class first, so you will be getting the minimum requirements... and yeah, there should be the total 75 points minimum from what I have read too. Well, there's one exception, if the kit has stat penalties/bonuses, like for example the avenger, these modifiers will be added on top of the min & max. So yes, you can make a char that min & max 3's in every stat, if you really want to.

Edited by Jarno Mikkola, 18 August 2016 - 02:17 AM.

Welcome to the sanity, you are free to search for the limit, it's out there, we drew it in the sand.
Here's how to install all the ... mods you ever really could want to Infinity Engine games. I removed the stable word from there as Roxanne began to add BS mods that are likely to break compatibility from the BWS.

#10 khelban12

khelban12
  • Members
  • 121 posts

Posted 17 August 2016 - 10:51 PM

1. Ok, did that.

:) Thank you once again.




2. Definitely something to test. With a human or something with low racial minimums and a similarly free class (vs. paladin's high requirements).

I did a small test. High difficulty (though i don't think it matters), Human Fighter, 75 rolls (i got bored after that :)). Results are the following: 92 (1 time), 85, 84, 82 (each 2 times), 83, 81 (each 3 times), 80 (5 times), 79, 78 (each 8 times), 77 (11 times), 76 (13 times), 75 (17 times). By the high number of 75 rolled and by the fact that nothing lower ever rolled, it seems that 75 is indeed the lower limit but my sample size isn't high enough to make a statistic conclusion. It isn't something crucial anyway, so even if gemrb doesn't match the vanilla behavior, it isn't big deal. I just mentioned it because i had done the change.
 

I've added it to the wiki research list that nobody seems to look at. Maybe I should sticky it here.

I follow the todo list and the rest of the wiki and i know you update it. I browse it from time to time to see if there is a topic i can help with but most are beyond my capabilities.
 

4. It's fine for few-liners, but how about uploading the patches to a pastebin next time? I could then apply them directly (most have a raw mode). But this one had no whacky spaces, so it applied in a here doc nicely. :)

Yes, pastebin is fine. Or maybe i should just use pull requests on GH instead of making your job difficult. I stopped using GH and made Gitlab my primary hosting on principle because i didn't like some decisions GH made, but that is not your problem and you shouldn't have to copy/paste diffs from fora/pastebin just because i don't like GH.


Edit: How could i forget about animation speed ?

A major problem i had was about enemies walking way too fast. I tracked it down to here where the movement speed is modified from 9 to the frame count of the animation. At first i thought that it was a problem with infinity animations but i can also reproduce it with the vanilla game (i have the impression that in older gemrb versions i didn't have this problem but i tried to go back to v0.8.0 or something and still everything moves too fast). For example the goblins in the starting dungeon move twice as fast as me (from their animation frame count they get a speed of 17). Disabling this code so that everyone has a base speed of 9 seems to work ok (i ran bgmain in wine and gemrb at the same time on the same save and characters move just a tad bit faster in gemrb so maybe the vanilla engine speed is 8 instead of 9 but i have no proof of that).

Edited by khelban12, 17 August 2016 - 11:39 PM.


#11 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 18 August 2016 - 02:02 AM

2. Good enough for me. :) I've modified the display though, so the title is preserved and not limited to English users. Thanks!

3. I actually meant this one, though the todo also evolved into having research topics as bugs were investigated: http://www.gemrb.org...opers:ietesting

4. Since you're active on the forum the combination with pastebin is ok. I guess we could also start a mirror on gitlab if you think that would be helpful.


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...


#12 khelban12

khelban12
  • Members
  • 121 posts

Posted 18 August 2016 - 02:57 AM

2. Good enough for me. :) I've modified the display though, so the title is preserved and not limited to English users. Thanks!

Yes my solution was sloppy. I just tested yours and it works perfectly.

I guess we could also start a mirror on gitlab if you think that would be helpful.

A new repo should be created on gitlab, all the people that contribute to gemrb should create a new account on gitlab, and everyone should change their workflow to accommodate me so that i can contribute 1 one-line fix every 3-4 months :). No, that is not acceptable. You have done more than enough already. I will do my modifications on GH and issue pull requests there.

Do you have any ideas about the walking speed of creatures ?

#13 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 18 August 2016 - 04:07 AM

Walking speed is one of our most annoying missing data problems. They were hardcoded in the animation code in the original and nobody extracted that. And of course, they were different amongst games (obvious with boots of speed in bg1 vs bg2). It was (partly? Short list) fixed in the ee version, but that's only one engine, one game: https://lynxlynxlynx...ee/extspeed.htm

 

Our fallback is 9, but we use the number of frames in the current animation cycle. A good approximation, but not good enough. Perhaps only a few animations need an override like it seems in ees, I guess that table would be an interesting start for exploration — which animation types do those inside use and what are their frame counts? Any patterns and so on.

 

Too bad we lost all our disassembly readers, it's all just black-box poking now.


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...


#14 khelban12

khelban12
  • Members
  • 121 posts

Posted 18 August 2016 - 09:08 AM

Walking speed is one of our most annoying missing data problems. They were hardcoded in the animation code in the original and nobody extracted that. And of course, they were different amongst games (obvious with boots of speed in bg1 vs bg2). It was (partly? Short list) fixed in the ee version, but that's only one engine, one game: https://lynxlynxlynx...ee/extspeed.htm
 
Our fallback is 9, but we use the number of frames in the current animation cycle. A good approximation, but not good enough. Perhaps only a few animations need an override like it seems in ees, I guess that table would be an interesting start for exploration — which animation types do those inside use and what are their frame counts? Any patterns and so on.
 
Too bad we lost all our disassembly readers, it's all just black-box poking now.


After the code that uses FrameCount, i added some code using AutoTable to load extspeed.2da and overwrite IE_MOVEMENT at least for those creatures in it and although it is not optimal (it opens the file once per every creature in the area), it seems to work. Can GemRB use this extspeed table or there will be legal problems with Beamdog ? For entries not in the file, i can fire both GemRB and bgmain.exe at the same time and try to visually estimate what speed is needed for each creature but it would be hit and miss.

If only Beamdog could release these kind of engine details so that disassembly of bgmain.exe and searching in the dark wouldn't be needed :(

Edited by khelban12, 18 August 2016 - 09:09 AM.


#15 lynx

lynx
  • Modders
  • 3144 posts
  • Gender:Male
  • Location:Ljubljana, Slovenija

Posted 18 August 2016 - 09:33 AM

We can use it, but we can't ship it without getting their explicit permission first. Did you decode the ids back to hex — if so, which class of avatars does it apply to? It looks too sparse to be affecting all player character choices.


GemRB - IE anywhere.
Mages needed! Looking for Planescape: Torment testers
Play android version IS NOT SUPPORTED ANYMORE: reported bugs will be ignored! Still looking for builders ...




Reply to this topic



  


0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users