Diminish quest mediator

[m9w]

No description provided.

[Pablete1234]

given the string caching that we do, and how otherwise string reading is an expensive operation, i'd say you can probably avoid doing this check at all.

Look into implementing separate update(addr) and update() methods, where the former invalidates some stuff and checks for this string, and this method checks if the address of the string it points to changes, if that is even required.

Is this an actual requirement or just a memory-bug fixing strategy?

[m9w]

It's solution that allow be sure that flash object was not removed by GC.
It's issue of flashmap updating where key removing from collection, bot foget about this instance and it caused that the existing memory pointer (address) is not actual can be reused by another object.

[Pablete1234]

that's something that the bot's dictionary for guis should already be handling, if the object stops being in the map it should be reset to address 0
i believe it was a bug that the older implementations for dictionary weren't doing it, but the new flashmap should be doing that just fine

[m9w]

It's great. So I'll remove this condition.

[m9w]

Done

[Pablete1234]

Suggested change

	if (address == 0) {
	state = State.NOT_EXIST;
	} else if (!"diminish_quests".equals(API.readString(address, 0x38))) {
	address = 0;
	state = State.NOT_EXIST;
	} else {
	state = State.NOT_EXIST;
	int stateIndex = API.readInt(address, 0x50, 0x40) + 1;
	if (stateIndex >= 0 && stateIndex < 5) {
	state = State.values()[stateIndex];
	diminishQuest.update(API.readAtom(address, 0x50, 0x60));
	}
	}
	if (!isValid()) {
	state = State.NOT_EXIST;
	return;
	}
	int stateIndex = API.readInt(address, 0x50, 0x40) + 1;
	if (stateIndex >= 0 && stateIndex < STATES.length) {
	state = STATES[stateIndex];
	diminishQuest.update(API.readAtom(address, 0x50, 0x60));
	} else {
	state = State.NOT_EXIST;
	}

i'd simplify the method in this logic, move validation check (addr != 0, and the other string check) into its own method, and make sure to check against the actual size of states instead of hard-coded 5
also just add a private static final State[] STATES = State.values() to avoid the allocations every tick.

Also, what's the +1? is there an extra state 0 in-game we're ignoring here?

[m9w]

Possible state indexes:

• -1 when quest not active (timer left etc),
• 0 when waiting accepting,
• 2 when quest accepted and active.
• 3 when quest done.

'+1' is mapping in-game index to enum index.

[Pablete1234]

Code looks much better now, just a few more simplifications and it's good to merge