Page 1 of 1

Kill counter

Posted: Tue Apr 06, 2021 2:09 pm
by Damrelia
I propose to add a counter for each killed monster in the info box next to the monster :)

Re: Kill counter

Posted: Tue Apr 06, 2021 2:22 pm
by Nut
Yes, sounds like a good thing.
Welcome to the forum!
Your name reminds me of Damerilias, the beautiful flowers from Remgard :)

Re: Kill counter

Posted: Thu Apr 08, 2021 6:07 am
by eor
I really like this idea. Could it be tied into the achievements "quest"? (i.e.; Arulir Assassin: 5000 killed,etc.)

It could be a nice diversion between releases to grind for "assassin" badges or achievements.

Re: Kill counter

Posted: Fri Apr 09, 2021 12:04 am
by Einsame Hirte
I've just about got this working and ready to pull if the team wants to use it. It shows up as "Previous kills" in the monster info box. Question about the string handling, though. I've added the appropriate fields to monsterinfo.xml with the default text, but should I also add it to strings.xml? Or is that generated programmatically somehow? (Never mind. It looks like it doesn't belong in strings.xml; I guess the translation stuff pulls it automatically).

And is this the appropriate place for source code discussions? Or better on github?

Re: Kill counter

Posted: Fri Apr 09, 2021 12:18 am
by rijackson741
Either, although IMO I prefer the discussion here.

Re: Kill counter

Posted: Fri Apr 09, 2021 2:33 am
by Einsame Hirte
Okay, I just made the pull request. Here's the (longer) text from the earlier one, since it's probably easier here. Any feedback appreciated.

kill count stats by monster name instead of ID #35
This patch fixes the "Most commonly killed monsters" display to prevent duplicate entries where more than one MonsterType has the same display name. It adds an additional hashmap to count on a per-name basis, while leaving the existing counter alone so that savegame files aren't affected. I tested it on the two versions of the "Iqhan chaos master" (red and purple).

This is both my first experience tweaking Java code and my first attempt to contribute to AT, so please let me know if I went to far in changing existing classes or refactoring existing code for clarity that should have been left alone - I could have minimized existing code changes by calculating on display only, but that would involve re-allocating an object each time and that's discouraged per the wiki. I think this approach is cleaner and will better support another patch I'm working on (adding kills to monster info display).

In particular, I'm not sure how the team feels about making Monster.monsterType public, but it enables playerKilledMonster() to pass the MonsterType object directly to the GameStatistics.addMonsterKill method (instead of just the string ID), which allows it to get the both the monster ID and name without an awkward double-lookup that would have required adding an argument to pass the WorldContext anyway. I know the extra cycles are insignificant these days, but trying to keep it efficient is a hard habit to break. Would it be better to add a Monster.getMonsterType() read-only property rather than simply making the variable public?

A note on potential wierdness due to this patch - I wouldn't go so far as to call it a bug. Because monster names are sourced from translations, it's possible that some monsters share the same name only in certain languages. This could potentially cause confusion if the kill counts change when the game language preference is changed.

Finally, one other potential concern. There is a killedMonster parameter in ConversationController.canFulfillRequirement that looks at kills on a per-monsterType basis. I haven't checked to see exactly how this is used, but it could potentially be confusing if the player sees a body count in one place (based on kills per monster name) and can't complete some conversation item based on a unique version of that monster. I don't think I've seen any actual gameplay situations where this would be a problem, though (i.e., a quest character saying "come back after you've killed kill 50 purple Iqhan chaos masters.")

Monsterstatus killcount #36
Adds "Previous kills" to the monster info activity. This is based on my previous killed_by_name branch and counts monsters based on having the same display name rather than same internal ID. See comments to pull request for that patch.