Feature - Implemented Use Effect Id / Item Id internally where possible

Currently Effect Name is used to look up effects internally. As a result we cannot handle effects with matching names, of which there are several.

EffectId is exposed whenever we encounter Effects, so they should be easy to handle there. In database files we will handle them as we currently do items, so if name is unique, we'll use name, if not unique we can use [xxxx]Name where xxxx is the Effect ID, to disambiguate them. [xxxx] will also be usable in ash scripts for specificity, or [name] will continue to work on a best endeavours basis.

First step, in r15508, is to make AdventureResult work with effectID constructors, and EffectDatabase to allow all lookups by effectId.

Next steps will involve going through anywhere using those constructors, comparions, lookups etc to prefer effectId, but work with effectName.

Then datafiles will be updated to use [xxxx]Name for ambiguous effects, and custom code for doing that job, or commenting out of things we can't handle, will be removed.


Something that started bugging me whilst doing r15508 was that we currently construct AdventureResults in one of two ways :
AdventureResult AUDITORS_BADGE = ItemPool.get( ItemPool.AUDITORS_BADGE, 1 );
AdventureResult AUDITORS_BADGE = new AdventureResult( ItemPool.AUDITORS_BADGE, 1, false );

Is there any good reason I shouldn't change them all to the ItemPool.get variant, or is the second preferred. Whilst I'm going through this I might as well make things consistent.

I could also create EffectPool.get( EffectPool.EFFECT, turns ) and use that for all Effect AdventureResults for consistency. Could then get rid of Effect enum section also.

Thoughts ?

I don't think I've ever used new AdventureResult() for items.

I always use ItemPool.get. It'd be fine with me if that is the only method that actually calls new AdventureResult() to make an item.

r15525. I think that's now Items only being made by ItemPool.get and Effects only being made by EffectPool.get.

Next I think I'll make them be only made by itemId and effectId (using ItemDatabase.getItemId( name ) and EffectDatabase.getEffectId( name ) first if necessary, as they handle both [xxxx] and NAME syntax).

Next move is to change all the existing databases that key on itemName to key on itemId instead, and effectName/effectId, whilst also making them read both [xxxx]Name and Name syntax for both.

Do not touch ConsumablesDatabase. As we've mentioned, that has things like hot dogs and chez snootee offerings and such which do not have ids. I have it on my own personal cleanup list for a variety of fixes.

Veracity said:

Do not touch ConsumablesDatabase. As we've mentioned, that has things like hot dogs and chez snootee offerings and such which do not have ids. I have it on my own personal cleanup list for a variety of fixes.

Click to expand...

Absolutely, there will be exceptions. Shall I just say I'll be converting anything I can.

There are also some things (like Card Sleeves) where KoL doesn't give us Item Id or Description Id so we have to use names.

r15530. Coinmasters database now uses itemId rather than itemName where possible.

Was briefly worried about non item objects, after making 95% of the changes, but the only ones there are no longer available. Hopefully Veracity will have time to do the pseudo item tidyup before KoL adds new ones.

I hope this is getting pretty much done now, at least in recognising items and effects being obtained, and in getting them from databases. I'm sure there will be some cleanup remaining, though.