Broken DB code

[Yay]

Hello,

I recently noticed that artifacts seemingly randomly lose their levels. Indeed they do not lose their level but the stats that are associated with the levels as well as the charges (and procs too I guess).

So I wanted to find out what the problem actually is; I compiled DOLServer; went into DOLConfig -> hacked in Database info -> test connection is ok -> go to server properties: Unable to connect/read SQL database. Old Version I used worked, so I naturally checked another old version.

I narrowed the problem down between 3582 (that worked) and 3583 (that doesnt and no successor version works).

Observations/Summary: I do not know if these two problems are related, but I cant debug it properly otherwise. I also noticed that the stats on the artifact are influenced by someone who logged on with a lower version. It seems it can only be downgraded but not upgraded while the server is running. (Caching issue I guess)

I'd like to find the error in code (especially in commits), but it's really not meant to be read, unfortunately.

Kind Regards,

YaY

[ontheDOL]

3582 was the last version i used before going on holidays... compiled 3595 last night and noticed the server properties in dolconfig unable to connect. I thought it must be my something wrong my end. Didn't see any forum posts with same issue (until now). Good to know.
sorry i realise this post doesn't help at all haha, but wanted to confirm the issue

[Velloro]

3583 introduced the database handler revamp. I imagine the issue may be related to that. Probably requires some actual adjustments to your tables to work. Are you getting error messages in the dolserver console window?

Here's a link from the commit. http://www.dolserver.net/viewtopic.php?f=59&t=22886

[Leodagan]

Probably something around ItemTemplate Caching...

It's one of the only table that use a static cache in runtime, this was a tricky feature to move to the new database handler, I'm not surprised that my implementation would break something

I'll try to look around to see why this is failing, but it's probably a DataObject refresh that pulls out an existing instance from cache...

[Yay]

Does that also explain why it's not possible to set up a fresh server? (Unable to read DB)

[Leodagan]

What do you mean by "not possible" to set up a fresh server ?

Database structure creation ? should be tested for each release (with SQLite driver at least)

The cache feature is not a driver feature but a "DOL" feature that keep records in a big collection and pick objects in this collection instead of creating a new DataObject for new queries... (only works with Select "All" and Select "by Id", Select "with where clause" cannot be cached this way...)

[Yay]

Ok, I think I pinpointed the issue.

• the actual artifact issue is solved by reverting the change in InventoryArtifact.cs from https://github.com/Dawn-of-Light/DOLSha ... a94209c965
• Setting up a fresh server didnt work for me because the Server couldnt convert NULL to 0 or EmptyString, so my fault. Did it manually and all is fine
• DOLConfig still doesn't work (that doesnt work since Revision 3853)

I actually didnt yet revert it, because I am not sure how to fix it without plain reverting it. Also I'm not sure if a bigger silent error lies underneath it.

[HunabKu]

All works on Windows and Linux for me with the latest git revision.

[Leodagan]

Ok, I think I pinpointed the issue.

• the actual artifact issue is solved by reverting the change in InventoryArtifact.cs from https://github.com/Dawn-of-Light/DOLSha ... a94209c965

Just a quick reply : you're brilliant, you've completely pinpointed it

The lines modified in InventoryArtifact.cs have a small comment just above it :

Code: Select all

// We want a new copy from the DB to avoid everyone sharing the same template

This is completely what I explained about cache !

I modified this line with the new API using FindObjectByKey instead of "Select with where clause", this was logic because the previous where clause was using the Primary Key for which we have specific methods that can take advantage of caching features !!

This line was intentionally using an un-optimized query to trigger side effect in Database Handler thus resulting in a new instanced object whatever the table caching status is...

This is what angers me about DOL, we can't rely on changing underlying implementations when most Code expect specific side effect to work !

We shouldn't revert this modification or the next time Database Handler move to another implementation we'll run into this bug again...

I think this specific artifact code should use the "Clone()" method to make sure he is handling a non-shared (or new...) DataObject instance, this would be more implementation agnostic

[Leodagan]

I tried some Patch, I'm really tired to fight with all these badly coded features, there is code for Artifact even in ItemTemplate source !

This is my proposal : https://github.com/Dawn-of-Light/DOLSharp/pull/36

[Leodagan]

And if anyone want to review the fix for Null to Not Null Migration this should be the one :

https://github.com/Dawn-of-Light/DOLSharp/pull/37