Lord Khelben's Bugs and Questions Thread

[khelban12]

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.

I don't think it has player character animations, just creatures.

 

If i didn't mess things up, these are the corresponding animations - speeds.

 

 

Animation ID	HEX		Creature		Frame Count	extspeed.2da
4096		1000		WYVERN_BIG		9		8
12288		3000		ANKHEG			21		6
25600		6400		DRIZZT			10		9
28672		7000		HALF_OGRE		10		7
28928		7100		BASILISK		13		6
28929		7101		BASILISK_GREATER	13		6
29184		7200		BEAR_BLACK		12		9
29185		7201		BEAR_BROWN		12		9
29186		7202		BEAR_CAVE		12		9
29187		7203		BEAR_POLAR		12		9
29696		7400		DOG_WILD		7		10
29697		7401		DOG_WAR			7		10
29698		7402		DOG_MOON		7		10
29952		7500		DOPPLEGANGER		10		7
29953		7501		DOPPLEGANGER_GREATER	10		7
30464		7700		GHOUL			10		5
30465		7701		REVEANT			10		5
30466		7702		GHAST			10		5
30720		7800		GIBBERLING		13		7
31232		7A00		SPIDER_GIANT		12		8
31233		7A01		SPIDER_HUGE		12		8
31234		7A02		SPIDER_PHASE		12		8
31235		7A03		SPIDER_SWORD		12		8
31236		7A04		SPIDER_WRAITH		12		8
31488		7B00		WOLF			7		10
31489		7B01		WOLF_WORG		7		10
31490		7B02		WOLF_DIRE		7		10
31491		7B03		WOLF_WINTER		7		10
31492		7B04		WOLF_VAMPIRE		7		10
31493		7B05		WOLF_DREAD		7		10
31494		7B06		WOLF_SHADOW		7		10
31744		7C00		KOBOLD_XVART		8		7
31745		7C01		KOBOLD_TASLOI		10		7
32000		7D00		ZOMBIE			12		5
32512		7F00		TROLL			10		7
32522		7F0A		GREATCAT_PANTHER	12		10
32523		7F0B		GREATCAT_LEOPARD	12		10
32525		7F0D		LICH			20		6
32527		7F0F		TROLL_SMALL		10		7
32557		7F2D		WYVERN_FAMILIAR		10		8
32559		7F2F		SPIDER_SMALL		12		8
32768		8000		GNOLL			14		8
33024		8100		HOBGOBLIN		14		7
33280		8200		KOBOLD			9		7
36864		9000		OGRE			14		8
40960		A000		WYVERN			9		9
53248		D000		EAGLE			7		10
57424		E050		LICHBLACK		18		6
57456		E070		MINOTAUR		16		8
57520		E0B0		TROLL1			18		7
58368		E400		GOBLIN_AXE		17		7
58384		E410		GOBLIN_BOW		17		7
58400		E420		GOBLINELITE_AXE		17		7
58416		E430		GOBLINELITE_BOW		16		7
58624		E500		LIZARDCASTER1		18		6
58656		E520					18		6
59136		E700		OROG1			18		7
59152		E710		OROG2			18		7
59168		E720		OROG3			19		7
59392		E800		ORC_MELEE1		19		7
59408		E810		ORC_RANGE2		19		7
59424		E820		ORC_MELEE3		19		7
59440		E830		ORC_RANGE4		19		7
59456		E840		ORC_SHAMAN		19		7

 

 

Is there someone in BD with the official capacity to permit use of extspeed.2da and/or to petition to him and ask about some engine details like these ones ? In the past they have released information about .itm,.spl and the other file formats, so maybe they are willing to share these details.

[lynx]

Nice, interesting, quite some jumps. One lich I tested now was definitely too fast and I remember the inverse for dogs or wolves (summon monster I). Trolls in the mound appear a bit fast too. It seems the guess was correct.

The only odd one out is the wyvern, but perhaps that was a poorly disabled tweak. Or maybe it's just my confirmation bias.

Mangling further, here are the counts vs animation types:

      3 2  IE_ANI_FOUR_FILES
      6 4  IE_ANI_CODE_MIRROR_2
      1 5  IE_ANI_SIX_FILES_2
      1 6  IE_ANI_TWENTYTWO
      1 7  IE_ANI_BIRD
      1 8  IE_ANI_SIX_FILES
     13 9  IE_ANI_TWO_FILES_3
      1 11 IE_ANI_FOUR_FRAMES
     30 14 IE_ANI_FOUR_FILES_2
      1 15 IE_ANI_CODE_MIRROR_3
      4 16 IE_ANI_TWO_FILES_3B
      1 17 IE_ANI_TWO_PIECE

So not many new types, meaning some of the data could also be useful for bg1 and iwd1. Once a base rate is found (boots of haste problem).

 

Their lawyers are the ones in capacity, but it would also be a complication for us (extra note/license), unless they release the file specifically under GPL to all. I'm not sure it's worth the trouble.

 

If anyone really wanted to confirm the theory that the frame count is king, that's pretty simple, especially now that we know which subjects/animations to avoid. Create a bunch of copies of creatures and their animations and just reduce the frame counts. Then you can spawn them, arrange them in a line and send them to the other side of the screen. Differences would be obvious. It would be even more obvious in ees, just by modifying the table.

[khelban12]

If anyone really wanted to confirm the theory that the frame count is king, that's pretty simple, especially now that we know which subjects/animations to avoid. Create a bunch of copies of creatures and their animations and just reduce the frame counts. Then you can spawn them, arrange them in a line and send them to the other side of the screen. Differences would be obvious. It would be even more obvious in ees, just by modifying the table.

That's how i wrote the "frame count" above and how i captured the animations i showed in the infinity animations thread. I used a for in weidu to copy a creature file N times each time altering the animation and put a print/Log call in gemrb to show the frame count.

 

When i get time, i will fire bgmain and gemrb at the same time and try to see the difference in speed for each animation (bgmain crashed every time i had more than 14 creatures in the area so it is a bit cumbersome to test). Maybe by visually comparing the difference, we can create a new table from scratch and avoid legal troubles.

 

Yesterday that i used extspeed to fill speeds for creatures, there was a weird thing. I think goblins in the starting dungeon (with their speed changed from 17 to 7) still moved a bit fast and additionally the movement of the sprites wasn't so "smooth". Is it possible that because of the smaller walk speed, some frames of the animation get dropped and not shown ? Or maybe my eyes played tricks on me. I will test it more thoroughly. I used SDL2 if it matters.

[Jarno Mikkola]

bgmain crashed every time i had more than 14 creatures in the area so it is a bit cumbersome to test

You should try and destroy them with the cheatkeys enabling and then Ctrl+Y.

[lynx]

This is easier to test with CreateCreature and MoveToPoint in a script — versus the console.

[khelban12]

What is the behavior of autopause ?

 

When i play a magic oriented character, i have the "spell cast" autopause option enabled. I am doing a BGT playthrough now and thus i have a rather heavily modded installation so i do not know if it is a gemrb bug or something in my installation but i have noticed three things in gemrb:

 

a) It doesn't work with all spells. I think that the only spellcast autopause i found is in a function named CreateProjectile. Does that mean that by design it works only with spells that have projectiles like e.g skull trap but not e.g stoneskin ?

 

b) I can't reliably trigger it. Even for spells that use projectile and i have seen it work, the game sometimes pause but sometimes don't (I have read that it only works in combat and all my tests are in combat situations).

 

c) It works with enemies. When Silke throws a lightning bolt at me, the game autopauses with "spell cast". Also there are many fights that have 5-6 spiders in it. Some difficulty mod have changed them to throw single target webs which i guess is coded as a spell, so i get 5-6 consecutive auto pauses which is annoying.

 

I have seen some "actor->InParty" and "actor->PCStats" checks in various parts of the code. I tried the following code using PCStats and it seems to work but as always i am blindly messing with code until it works and that is why i didn't issue a pull request. If this is the proper action to take, then i will issue a pull request.

 

 

 

diff --git a/gemrb/core/Scriptable/Scriptable.cpp b/gemrb/core/Scriptable/Scriptable.cpp
index c897c55..d1b672b 100644
--- a/gemrb/core/Scriptable/Scriptable.cpp
+++ b/gemrb/core/Scriptable/Scriptable.cpp
@@ -908,7 +908,10 @@ void Scriptable::CreateProjectile(const ieResRef SpellResRef, ieDword tgt, int l
 	}
 	// only trigger the autopause when in combat or buffing gets very annoying
 	if (core->GetGame()->CombatCounter) {
-		core->Autopause(AP_SPELLCAST, this);
+		Actor *caster = ((Actor *) this);
+		if (caster && caster->PCStats) {
+			core->Autopause(AP_SPELLCAST, this);
+		}
 	}
 
 	gamedata->FreeSpell(spl, SpellResRef, false);

 

[SyntaxError]

You can't just arbitrarily cast a Scriptable to an actor; thats a great way to induce a crash or other bad behavior. You need to check its type: caster->type == ST_ACTOR

 

I'm also a bit confused to why you are accessing PCStats since you don't do anything with it... but on a non actor accessing that field is going to point to something else and it may or may not be "truthy", which is all you are checking. Additionally the check for caster truthiness is meaningless. `this` should never ever be NULL in C++ (says so in the standard).

 

to see this crash/not work correctly just trigger a projectile trap.

 

A last nitpick please don't use C style casting. I know the code is littered with it, but IIRC thats due to the C roots of the project. C style casts are the ultimate in "trust me" programming. Use a C++ static_cast to cast within an inheritance hierarchy where you know the type (you do though caster->type).

 

Thank you for looking into it, however. We always appreciate helpful assistance.

[khelban12]

I'm also a bit confused to why you are accessing PCStats since you don't do anything with it... but on a non actor accessing that field is going to point to something else and it may or may not be "truthy", which is all you are checking. Additionally the check for caster truthiness is meaningless. `this` should never ever be NULL in C++ (says so in the standard).

I have no idea why I wanted a way to trigger the code only for PCs/NPCs.

 

See for example this code. It does the same check and then it registers a favorite spell. Doesn't this look (at least to someone not familiar with gemrb) like a check to discriminate between PCs/NPCs (who have favorite spell in their character record) and other creatures ? I saw some checks like this one or others that used actor->InParty in many parts of the code and i thought that this is the way.

 

Because i ask many things, i try to do my homework and provide patches instead of burdening lynx with doing all the work, but many times it leads to post irrelevant/sloppy code that happens to work like the above. Sorry for the noise.

[SyntaxError]

The code you referenced the caster is not always "this" so that is why it must check for NULL.

 

PCs/NPCs should always be "Actors" so checking the type should get you that without needing to access PCStats.

 

A lot of this logic is due to the quick port from C to C++ so we kept a lot of the old conditionals in the base class instead of leveraging polymorphism like we would have done if this were originally C++ code.

 

Its not noise it is helpful even if we have to clean it up a bit; no apology necessary!

[lynx]

a) Could be, yes, for anything that just applies its effect queue directly. Autopause markers were added one by one, so it's very conceivable that some spots are still lacking them. It's not something anyone of us uses in our test runs or the feature would have been added much sooner. So new checks are fine, we just have to be careful not to start spamming (like with enemies).

 

How did it work in the original? Any player spellcasting triggered it?

 

b) Test case?

 

c) Since it seems the original did limit it to the party, the approach is correct. The difference between checking InParty and PCStats is subtle. One is for party members (actors with a portrait in the window), but PCStats are there for more (limited by player classes). However, neither are perfect in this case. Some, if not all, familiars also have spells, so they should get the same treatment, but they're not considered part of the party. So the perfect check would be against Actor::IsPartyMember() .

 

GemRB has always been in C++, but nobody used much of the complexer functionality due to poor compiler support and/or ugly generated code. The situation has luckily improved.

[khelban12]

a) Could be, yes, for anything that just applies its effect queue directly. Autopause markers were added one by one, so it's very conceivable that some spots are still lacking them. It's not something anyone of us uses in our test runs or the feature would have been added much sooner. So new checks are fine, we just have to be careful not to start spamming (like with enemies).

 

How did it work in the original? Any player spellcasting triggered it?

The original engine triggered it for all spells cast by the party. Buffing was indeed annoying so your version is better.

 

b) Test case?

I can't reliably produce a test case. Some spells seem to not trigger the pause all. For example doesn't Magic Missile use a projectile ? Enemies have fired a ton of magic missiles at me (and i at them) and i never remember auto pause triggering. Others spells work intermittenly. For example an enemy fires a lightning bolt at me and the autopause triggers and 2-3 rounds later he fires another bolt at me and it doesn't trigger.

[lynx]

I couldn't reproduce any problems and also tried magic missile. Also judging from the code, all successful combat casting should trigger it already. Something to keep an eye on.

 

I've added the party check.

[SyntaxError]

 

 

GemRB has always been in C++, but nobody used much of the complexer functionality due to poor compiler support and/or ugly generated code.

 

My whole life is a lie! I thought I remembered having a conversation with Fuzzie about why (much of) the code more resembles C than C++... Polymorphism shouldn't have fallen into the category of poor support. Now I cant fathom why there are so many type checks in the base classes instead of polymorphic overrides and template methods (not to be confused with C++ templates).

 

Oh well.

[khelban12]

Here we can read that malloc requests 4K memory per line so it can cope with table files with long lines.

 

Will it have any adverse effect on phones and stuff with not much memory if this is changed to 8K (a few lines after the malloc, realloc is ran and "shortens" the line so the increase will be temporary) ?

 

I had a problem with my previous BGT run but i didn't have time to search it. Every spawn point gave me an enormous amount of enemies.

 

For example, in the area right after leaving candlekeep you get a spawn point with i think a gibberling and a xvart and i got maybe ten gibberlings. In other places where the vanilla engine spawned two wolves, gemrb spawned 6+ wolves. I even got 3 trolls somewhere. In the area i am now i got 5 bandits and 5 gibberlings where the vanilla engine only spawns 1 of each.

 

Anyway, to make the story short, i tried to find why this is happening and found that spawngrp.2da is not loaded correctly. Adding printfs showed that GetRowCount reports 19 lines (when it should report 9) so i thought that lines from the file are split at some point and everything gets messed up. I traced it to the above limit of 4096 bytes per line.

 

spawngrp.2da as it is modified by BGT has 6365 bytes per line (even after trimming the extra spaces weidu puts with pretty_print_2da it only goes down to 5211) so 8192 should be a safe value.

 

After changing it to 8192, i get the correct number of enemies.

 

Edit: I forgot to mention another thing i noticed. In candlekeep there is a quest where you need to eliminate some rats. In the vanilla engine, although it says "rat attacks charname", i don't remember ever getting hit even if i left the game unpaused for minutes. In GemRB, the rats attack normally. The area (ar6506 in my bgt) file says that all the rats use the rat2 creature file which has an APR of 0.

 

In Actor.cpp there is this code that ensures the APR is always at least 1. It was added in this commit so that slow works properly. If i comment this code, then the rats behave as the vanilla engine and cannot attack.

 

This is not so important, i just mention it because i remembered it now. I guess that slow behaving correctly is more important than one creature file that is only encountered once in the whole game.

[lynx]

Another nice find, good work. The increase is fine, the rats annoying. In other cases they used no weapons, so they would only do stunning damage. Try this: http://paste.debian.net/828352/