Serious Core bug :-(

[OWHorus]

Hello,

after having the newest core productive for several days I found a serious bug *sigh*

Does nobody TEST the cores? This core is unchanged since March 2016 - obviously nobody uses it, or am I wrong??

The problem:

Reading the AOS resistances and damages goes totally wrong - it doubles the values defined in npcdesc.cfg. Here is an example, tested with an empty world file, and with our npcdesc.cfg:

Here we have the entry for a dragon in our npcdesc.cfg:

Code: Select all

// Original NPC entry
NpcTemplate N003b
{
 Name <DRAGON>
 Title Drache
 Script simplenpc
 GermArt 1
 PoisonResistance 100
 Behaviour LootMonster

 ObjType 0x003b
 Icon 0x20d6
 TrueColor 0
 Gender 0
 STR 1d30+795
 INT 1d40+435
 DEX 1d20+85
 Runspeed 40
 NpcLevel 3810

 Parry 1d40+55
 MagicResistance 1d2+98
 Tactics 1d2+98
 Wrestling 1d3+90

 AR 50
 AttackDamage 2d12+4
 AttackSpeed 30
 AttackAttribute Wrestling
 AttackHitScript :combat:npchitscript

 FireResist 750
 ColdResist 50
 PoisonResist 250
 FireDamage 10

 Alignment evil
 IdleSound 0x016c
 AttackHitSound 0x016d
 DamageSound 0x016e
 DeathSound 0x016f
 AttackMissSound 0x0234
 }

Of interest are only the AOS resistances. Do not mind the values - we use a complete attack hook, and we know how to use this values. The attack hook, and the resistances exist now for years, and work well with POL 0.99, hence the last release we used was from around 2012. (Here you see one reason we rarely update the core. I sat for days just to rewrite things which were changed during the last 4 years, often with no obvious reason.)

So - our dragon should have 750 fire resistance, 50 cold resistance, 250 poison resistance, no energy resistance and do 10 fire damage. And it has an AR of 50. We still use AR, hence AOSResistances are zero in uoclient.cfg. We also use hit zones and AR zones, but AR is not the problem here, this works well.

I used the following tiny testscript to read the values from our new NPC directly:

Code: Select all

program testvalues(who)
	var mobile := Target(who, 0);
	if (mobile)
		if (mobile.ar)
			print(mobile.ar);
			print(mobile.resist_fire);
			print(mobile.resist_cold);
			print(mobile.resist_poison);
			print(mobile.resist_energy);
			print(mobile.damage_fire);
			print(mobile.damage_cold);
			print(mobile.damage_poison);
			print(mobile.damage_energy);
		endif
	endif
endprogram

The script gives the following output on the console (POL runs in x64 under Windows 7 for this test, but on x64 Linux the problem is identical).

Code: Select all

50		AR
1500		fire resist
100		cold resist
500		poison resist
0		energy resist
20		fire damage
0		cold damage
0		poison damage
0		energy damage

As we can see (I have other tools to show these values, and they show the same) the resistances _and_ the elemental damages are doubled. There are no mods set, all the mods are zero. We do not use mods to elemental resists/damage on NPCs. We sometimes use AR mods, they work fine. And you can see, that AR is read and set correctly.

So okay - maybe its a small core bug, and we can catch it in the scripts, by dividing the values by 2? Well - yes, BUT...

Here is the save entry of this dragon in npcs.txt:

Code: Select all

// Save in npcs.txt, after create
NPC N003b
{
	Name	Volan, der Drache
	Serial	0x20a3
	ObjType	0x3b
	Graphic	0x3b
	X	2777
	Y	880
	Z	0
	Facing	6
	Revision	1
	Realm	britannia
	CProp	Typ i7
	TrueColor	0x0
	TrueObjtype	0x3b
	Gender	0
	Race	0
	Strength	800
	Intelligence	451
	Dexterity	98
	Parry	74
	MagicResistance	99
	Tactics	100
	Wrestling	92
	Life	800
	Mana	451
	Stamina	98
	CreatedAt	0
	AR	50
	script	simplenpc
	SpeechColor	37
	RunSpeed	40
	FireResist	1500
	ColdResist	100
	PoisonResist	500
	FireDamage	20
}

So - the save stores the wrong (doubled) resistances and damages. This was to expect...

Okay - POL goes down, saves, and is started new, reading its saves, including our dragon.

After it is up, I log in and use my small testscript to check our dragon, and this is the output:

Code: Select all

// Output of testscript after stopping the core, restarting and loading the save
50		AR
3000		fire resist
200		cold resist
1000		poison resist
0		energy resist
40		fire damage
0		cold damage
0		poison damage
0		energy damage

UPPPS...

So - every time we have to restart our system, all NPCs with resistances and damages not zero have DOUBLED values!!!

After stopping the POL core again, the save entry for our dragon looks now like this

Code: Select all

// Nnpcs.txt - save after going down again
NPC N003b
{
	Name	Volan, der Drache
	Serial	0x20a3
	ObjType	0x3b
	Graphic	0x3b
	X	2775
	Y	871
	Z	0
	Facing	1
	Revision	1
	Realm	britannia
	CProp	Typ i7
	TrueColor	0x0
	TrueObjtype	0x3b
	Gender	0
	Race	0
	Strength	800
	Intelligence	451
	Dexterity	98
	Parry	74
	MagicResistance	99
	Tactics	100
	Wrestling	92
	Life	800
	Mana	451
	Stamina	98
	CreatedAt	0
	AR	50
	script	simplenpc
	SpeechColor	37
	RunSpeed	40
	FireResist	3000
	ColdResist	200
	PoisonResist	1000
	FireDamage	40
}

You can calculate how long it takes until our dragon is completely invulnerable against any elemental damage AND does so much elemental damage itself, that it can overtake our shard.

I am NOT amused. How could this slip through????

Now I sit with a productive shard, players online and nearly 4 days gaming in our save and have to find out this.

I am forced to delete ALL our monsters before I put down the shard, or we will have a completely unusable shard.

Could you please look into this real FAST?? I cannot believe that nobody saw this for 5 Months now...

I also tried to recompile my core (just hours before) for Windows, under Windows 7 and with VC 2013, without problems, and the core compiled under Debian (our productive core) has the same behavior. So I am quite sure it is a bug. Documentation says nothing about changed behavior in this regard.

Here is my guess:

There are two new and shiny functions in npc.cpp:

void NPC::loadResistances( int resistanceType, Clib::ConfigElem& elem )
void NPC::loadDamages( int damageType, Clib::ConfigElem& elem )

Both do this:

Code: Select all

...
  switch ( resistanceType )
  {
  case 0:
    npc_ar_ = static_cast<u16>( value );
    break;
  case 1:
  {
    if ( value != 0 )
    {
      fire_resist( fire_resist().addToValue( static_cast<s16>( value ) ) );
      curr_fire_resist( curr_fire_resist().addToValue( static_cast<s16>( value ) ) );
    }
    break;
  }
...

If I read the code (also in dynproperties.h) correctly, it always adds the value:

Code: Select all

inline AosValuePack& AosValuePack::addToValue( s16 other )
{
  value += other;
  return *this;
}

So - we first read the value, then use Core::Dice dice() on it (which leads always to the same values, since there is only a constant part), and then we _add_ the value again, which doubles it. Except if the value is zero - then nothing evil happens.
The same seems to happen, if we read the resistances from the save (npcs.txt). I suspect, this is the reason.

Maybe we need a new inline method:

Code: Select all

inline AosValuePack& AosValuePack::setToValue( s16 other )
{
  value = other;
  return *this;
}

But I may be sadly mistaken - if I find the time, I will try it...

OWHorus

EDIT:
I just checked - items (equipment has several set resistances and damages) are stable - thanks to god, because it would have been awful to replace these all. But they are loaded correctly, and stored correctly...

OWH

EDIT2:
I seem to have a solution, but I doubt, that it is the correct one...

I added a new inline function - see above - and use it to set the curr_xxx_resist and curr_xxx_damage values. The rest is unchanged. As a consequence newly created NPCs have now the correct values, and store / load correctly.

There seems to be a mix up between the Template values, the 'real' values which are used and the mod values. I further experimented with the unchanged core (without my hack), and found, that if you set a mod for example to fire_resist, the value you read back in script is changed correctly, the mod value is stored correctly too - but the fire_resist value in the save has now the _new_ changed value. If you save, stop the server, and reload it from the save the NPC has again the incorrect value, since the mod is read back from the save, but as base value the stored _changed_ value is used. So the mod is added twice (it is contained in the world file, and calculated into the resist value, but not taken into account during load).

I doubt that this works with my hack, there is more at fault. But since we do not use AOS-Resistances with mods in our scripts, this will work. (Also NPCs do not use equipment which will change resistances).
The resistances of items are correct, loaded from the world save or created new. Also the character is updated fine if he equips things which change his resistances.

But somehow I have the impression, that this part of the core is not ready.

I really hope, that you find the time to look into that. It was my impression, that the core has a stable state, and you are waiting for feedback and will release it.

OWHorus

[Yukiko]

OWHorus I copied your Drache template to my NPCDesc file and tested this. I got slightly different results. I am running the latest Core from the Git repo. I'm using a modified info command that shows elemental resistances. In game I looked at the elemental resist values and they are indeed doubled from the NPCDesc file.

As I understand from your posted examples of your NPCDesc entries POL is saving the doubled values to your NPCDesc file each time you shut down the server. So that when you restart, it loads the doubled values and again doubles them and then saves the newly doubled values again and so on.

So I started POL and created the Drache. On my system the original values still remain in the NPCDesc file after shutdown. However, each time I shut down POL (using Ctrl-C) and restart POL the values on the Drache show up in game as doubled from the last time I ran POL, So the first time I ran POL Fire Resist was 1500 (double the entry in the NPCDesc file). I shut down POL and restarted it and the Fire Resist was 3000 (quadrupled from the entry). Shut down and restart and now the value is 6000 (octupled). This is if I leave the Drache "alive" and shut down and restart POL. If I create a new Drache it is created with the values from the NPCDesc file but doubled, ie. Fire Resist is 15000 not 750 as in the Desc file. There is clearly a problem with the Core.

Keep in mind I am running on Windows. As I recall you are running on Linux. Also, my NPCDesc file is in \pol\config as it used to be in the pre-0.96 Distro. That might explain why POL isn't modifying my NPCDesc file. There might be some built-in protection that won't alloe POL to change files in the \pol\config directory. If yours is under the \pol\pkg directory, presumably \pol\pkg\npc or some such location that might allow POL to access and change your Desc file. It shouldn't but that could explain the different results.

Thank you for finding this bug!
I am not a Core developer but hopefully minds greater than mine will fix this.

[Yukiko]

A follow-up report.

This behaviour doesn't happen on Player Characters. It seems to be limited to NPCs.

[OWHorus]

OWHorus I copied your Drache template to my NPCDesc file and tested this. I got slightly different results. I am running the latest Core from the Git repo. I'm using a modified info command that shows elemental resistances. In game I looked at the elemental resist values and they are indeed doubled from the NPCDesc file.

As I understand from your posted examples of your NPCDesc entries POL is saving the doubled values to your NPCDesc file each time you shut down the server. So that when you restart, it loads the doubled values and again doubles them and then saves the newly doubled values again and so on.

So I started POL and created the Drache. On my system the original values still remain in the NPCDesc file after shutdown. However, each time I shut down POL (using Ctrl-C) and restart POL the values on the Drache show up in game as doubled from the last time I ran POL, So the first time I ran POL Fire Resist was 1500 (double the entry in the NPCDesc file). I shut down POL and restarted it and the Fire Resist was 3000 (quadrupled from the entry). Shut down and restart and now the value is 6000 (octupled). This is if I leave the Drache "alive" and shut down and restart POL. If I create a new Drache it is created with the values from the NPCDesc file but doubled, ie. Fire Resist is 15000 not 750 as in the Desc file. There is clearly a problem with the Core.

Keep in mind I am running on Windows. As I recall you are running on Linux. Also, my NPCDesc file is in \pol\config as it used to be in the pre-0.96 Distro. That might explain why POL isn't modifying my NPCDesc file. There might be some built-in protection that won't alloe POL to change files in the \pol\config directory. If yours is under the \pol\pkg directory, presumably \pol\pkg\npc or some such location that might allow POL to access and change your Desc file. It shouldn't but that could explain the different results.

Thank you for finding this bug!
I am not a Core developer but hopefully minds greater than mine will fix this.

Hello Yukiko,

thank you for checking this and for replying!

We too have the npcdesc.cfg in config/, but this should not matter. The difference is, that you can unload packages, so you can have changing npcdesc.cfg in a package for special NPCs. The npcdesc.cfg in config/ cannot be unloaded, if you change something a reboot of POL is necessary. As I understand the documentation, the npcdesc.cfg can live in both places, only the official distro is often used as a template. We used the distro as a base for our work (back in 2002 and 2003) and therefore have an older file structure, but in the meantime all our scripts are our own, we have no traces from the distro left, neither newer or old.

I am satisfied that you also see this problem - I was a bit unsure and questioned myself if I had made an error...

The behavior of POL under Windows and Linux is identical. I use Windows for my daily work (Microsoft tries hard to change this ) and Linux on our game server. So I always had 2 versions of POL, one for the server, the 'productive' version and one for me to test things at home. (I also have a VM with Linux here).

The problem is serious, because it cannot be fixed or worked around with scripting. The mobile.resist_xxx members are read only and cannot be changed by script, only mobile.resist_xxx_mod is writeable. The same goes for elemental damages.

I also found, that characters (i.e. player characters) work normally, changing equipment will change the resistances and damages. Since NPC is a derived class from character, the mix up happens somewhere in NPC code. Sadly I do not understand the intentions and structures in POL code well enough for a real fix. So I made this hack, which seems to work. But - we do not use the core attack, we use a completely own attack code in script (the attack hook script) and so we use only the values. NPCs in our system never have changing resistances or damages because they use no equipment. But if the values change with every reboot and are wrong from the start we do have a problem.

I tried to test with my test shard at home and my character went down in one strike - because I had often rebooted this test shard for testing of several changes the NPCs had their resistances maxed out (resistances use a 16 bit integer so they were all around 50000-60000).

I also tried to 'fix' the problem with mods - you could mod a NPC if it should have 750 and has 1500, you can set a mod with -750. It is a bit awful, but works - but in every save the _new_ resist value and the mod is saved. So the save has then 750 resist and -750 resist_mod. If you load this save, the NPC has afterwards 0 resists (saved value + saved mod). There is no possible way to work around this in scripting...

So - if you need my hack, I will send it to you (private message). I do not want to post it, because while it works I am unsure if it will work under any circumstance, and if people use mods I rather doubt it will work correctly. The hack is simple, only two files are changed, and it compiles without problems, but that doesn't mean it is correct...

I am a bit unsure how core development goes at this time - there are no changes since March. I really believed they are waiting for feedback and want to release the core 0.99. They last changes are only bug fixes. So I thought to use this core since it has a recent feature we need. I just hope there are no other sleeping trolls in the core...

But - just to say it, because it needs to be said: The compilation process of POL under Windows and under Linux is now working like a charm!! There are no doubts what you need and if it works. One command and the core compiles and the archives with the distro are built. This is really nicely done and a relief to earlier processes!

I am also a bit astonished that nobody found this problem, it is rather obvious if you run a shard with resistances. Maybe many shards use older clients (we use UO-ML, version 5.06, which is not exactly new) without elemental resistances, in this case you have no problems with it.

I hope the the original developer of this code has the time to look into it and provide a real fix, not a hack...

So give me a PM or say so here if you want the hack...

OWHorus

EDIT: I just reread your post - there seems to be a small misunderstanding: The values in npcdesc.cfg were never changed in my tests. What happens is the same behavior you see too: A newly created NPC has doubled values, and with every _load_ of the save (which happens only if you reboot the shard) the values again double. If POL starts to modify my config, I would have been deeply shocked - this would be really awful.
But I know no shard who kills/deletes all its monsters with a reboot. But this would be the only way to limit the wrong resistances to the double values at least. Every monster that exists during reboot will have its values doubled again and again with every reboot. But npcdesc.cfg remains unchanged. AFAIK POL never writes config files except per script changes in packages, but this is the responsibility of a special scrip. But the normal mechanisms only read config files.

OWH

[Turley]

Didn't had much time but the copy and paste error is fixed.

[Yukiko]

Thanks OWHorus. I PMed you but I will repeat one thing I said in my PM. The Core source files are sorely lacking in comments. I don't know how anyone who wants to understand how POL works internally and possibly help with Core development can easily get started. I don't know how active the Core team is right now either.

As for the testing of the Core releases, I don't know if "normal" shard admins are testing new releases because it involves compiling the Core. That requires installing compilers and such. I suspect that they are kept busy enough just running the shards and writing and debugging scripts. I haven't looked to see if there's a ZIP file with the latest release posted on the forums. It's too bad we don't have an automated process for releasing a compiled package whenever there's a new commit. I have to assume that system would require a lot of work and that's probably why it's never been created for POL. Also I am not sure when this bug got introduced,

My shard is pretty much just a hobby for me now as I don't have a playerbase to speak of. I program and lurk on the forums just to keep my brain active and to help whenever I can. I have been adding resistances slowly but there's always some new thing I want to play with in scripts that draws me away.

As with you, my directory structure is much like the old Distro structure but I am trying to adopt the new layout to some degree. Austin's layout for the 0.96 and higher script releases is better organized in many ways.

I wasn't aware that you could unload a pkg and have POL reread the NPCDesc file from the package while POL was active. I thought all NPCDesc and itemdesc files were loaded at startup and couldn't be reloaded properly with POL running. Good to know this.

The behavior of POL under Windows and Linux is identical.

Hahaha! Yeah. It's only the OSes that behave differently. I say this after having to fix the Distro_Alt to comply with Linux's case sensitivity when dealing with file names.

And yes I am getting annoyed with Microsoft's insistence on just restarting Windows without my permission after an update. Although I think I solved this by modifying the group policy.

Anyway, it looks like Turley is hot on the trail to fix this bug if he has time. So there's hope.

[Turley]

Anyway, it looks like Turley is hot on the trail to fix this bug if he has time. So there's hope.

It was already correctly when I wrote my post.
Checkout github :p

[Yukiko]

Turley Turley he's our man!
If he can't do it nobody can!

Thanks.

[Turley]

Lol thanks.
About comments: most of the code is from a design perspective pure horror and error prone. We do our best to rewrite the stuff and comment it. But like this bug of cause it has the possibility to add new stupid bugs...
Automated builds are currently only possible for linux, the windows build takes to much time on appveyor. And it has the downside that we need to store somewhere all the debug symbols so we can react on crashes. This would flood shortly every server if we do this for every commit.

[Yukiko]

most of the code is from a design perspective pure horror...

It sounds like Stephen King wrote some code.
I didn't know he was a coder.

I understand commenting code is usually an after thought. I hate commenting stuff. after all I know how it works so why should I comment my own code. Then I come back to it a year later and think "What in creation does this do?" and wish I HAD commented it. Please understand I am not complaining about Eric or the later developers' work on POL. It's an amazing "machine". It's given me years of pleasure, frustration, heartache, and joy working with it. You guys are amazing coders! When I think about all the flexibility built into POL it's a marvelous environment to script for. The Distro developers have done some awesome work from the very start as well. I just wish there were some sign posts in the code to aid those like me who are ignorant and want to learn so we can help with bug finding if nothing else. So maybe as you look through code from time to time you can drop some few comments in and maybe, over time, the Core source will be more understandable for us newcomers.

Thanks again to all the devs, both Core and Distro, for your work.

[OWHorus]

Hello,

bug is fixed in newest core:

I tested to create a NPC, and it had the correct values. World save has the correct values too, and loading the world save leads to a NPC with correct values.

I also tried to set a mod (with damage_cold), and npc.damage_cold returns the base value plus the mod. In the world save both values, the base value and the mod value are stored. If this save is loaded, the NPC has its correct values including the mod, the mod is loaded correctly too. The mod can be read from the NPC too, and is correct. If I remove this mod (by stetting it to zero) the base value is on the NPC again and is returned by npc.damage_cold.

I did not test it with all possible resistances or damages, but I think it works now as intended.

Thank you for fixing it so fast

OWHorus