-
Notifications
You must be signed in to change notification settings - Fork 252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improved database persistence for biota_properties_generator #2616
base: master
Are you sure you want to change the base?
Conversation
Should something similar be applied to the WorldDatabase where the weenies are read? |
Technically when a bunch of the of the weenie* tables are read in, it should be ordering by id, ensuring the records are returned in the order they were entered into the db. The World DB does not have the same problems as the Shard DB though. The Shard DB biota tables are written incorrectly, as a result of EF writing collections / sets in its own internal way, providing no interface to specify or guarantee the ordering. So if the question is 'should we also be sorting the weenie generators table by probability when it is read in', the answer is no, most definitely not. In fact, we shouldn't really even be doing it for the shard database -- it is only a hack to avoid having to place the order column on the biota_generators table. The order column would be the 'real' way to do it. |
@@ -223,7 +223,7 @@ public static Biota GetBiota(ShardDbContext context, uint id) | |||
if (populatedCollectionFlags.HasFlag(PopulatedCollectionFlags.BiotaPropertiesEnchantmentRegistry)) biota.BiotaPropertiesEnchantmentRegistry = context.BiotaPropertiesEnchantmentRegistry.Where(r => r.ObjectId == biota.Id).ToList(); | |||
if (populatedCollectionFlags.HasFlag(PopulatedCollectionFlags.BiotaPropertiesEventFilter)) biota.BiotaPropertiesEventFilter = context.BiotaPropertiesEventFilter.Where(r => r.ObjectId == biota.Id).ToList(); | |||
if (populatedCollectionFlags.HasFlag(PopulatedCollectionFlags.BiotaPropertiesFloat)) biota.BiotaPropertiesFloat = context.BiotaPropertiesFloat.Where(r => r.ObjectId == biota.Id).ToList(); | |||
if (populatedCollectionFlags.HasFlag(PopulatedCollectionFlags.BiotaPropertiesGenerator)) biota.BiotaPropertiesGenerator = context.BiotaPropertiesGenerator.Where(r => r.ObjectId == biota.Id).ToList(); | |||
if (populatedCollectionFlags.HasFlag(PopulatedCollectionFlags.BiotaPropertiesGenerator)) biota.BiotaPropertiesGenerator = context.BiotaPropertiesGenerator.Where(r => r.ObjectId == biota.Id).OrderBy(r => r.Probability).ToList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your reasoning makes sense, and this change should work.
I'd like to see a comment above this line though. It should describe why we're doing the OrderBy here. It would also act as an indicator to note that this line is different among all the other queries.
In the future, if we do add an order column to this table, we could get caught out by not realizing this band-aid exists.
I did a search through all the weenie_properties_generators, and for every object except 31986, the probabilities are always in ascending order.
However, this is not 100% foolproof, and there are a few caveats with this: some generators contain spans with the same probabilities, usually a bunch of -1's... these will still be sorted into a non-guaranteed order
This should cover 99.9%+ of the scenarios though, and let us get away with restoring the biota_properties_generator in the original weenie_properties_generator ordering, without an explicit 'order' column