Bug - Fixed Recent change caused loss of useful strategy information

[DoctorRotelle]

Request 37 of 256 (The Red Zeppelin's Mooring: A Mob of Zeppelin Protesters) in progress...

[19715] A Mob of Zeppelin Protesters
Encounter: eagle
Strategy: C:\Documents and Settings\Administrator\My Documents\Mafia\ccs\stoppers.ccs [eagle]

OK, that [eagle] part should probably have been placed on the previous line (and omitted when it was an actual match to the "encounter" monster name).

The previous information contained in the square brackets used to say which strategy was chosen from the ccs. Now it is misleading and confusing. I can't tell you how many times I panicked thinking I had a special section for a monster and got confused when I couldn't find the section in my ccs. Now I have no clue which section is being triggered.

 

[lostcalpolydude]

The previous information

When did mafia last have that information?

 

[DoctorRotelle]

Looking at the svn log, I'm guessing it was this change in revision 14243 to FightRequest.java

@@ -1764,7 +1765,7 @@
 			if ( action.startsWith( "custom" ) )
 			{
 				String file = Preferences.getBoolean( "debugPathnames" ) ? CombatActionManager.getStrategyLookupFile().getAbsolutePath() : CombatActionManager.getStrategyLookupName();
-				action = file + " [" + CombatActionManager.getEncounterKey( MonsterStatusTracker.getLastMonsterName() ) + "]";
+				action = file + " [" + FightRequest.getCurrentKey() + "]";
 			}
 
 			RequestLogger.printLine( "Strategy: " + action );

I'm reverting my personal copy to test out after rollover. During the Twitch event this Sunday I'll depend on the information since I've changed combat strategies for each of my multi's too many times for me to track without. And tomorrow is the start of the new series of Doctor Who. Fixing my ability to do svn commit's is low on my priority list, so I hope you or veracity can follow up on this.

 

[Darzil]

I nearly missed that one, for a while I thought all that had changed was one function pointing to another.

However, it turns out that there is a major difference between encounterKey( monstername ) and getEncounterKey( monstername ). The former returns the encounter strategy (ie cleaned up monstername), the latter returns the BEST encounter strategy.

r14471 reverts that change.

I don't have time to investigate further now, but I suggest we change those CombatActionManager function names so they are harder to confuse, and make sure getCurrentKey() in FightRequest is getting the right one too.

 

[Veracity]

This is fixed, right? Or is this still open because of the suggested refactoring/renaming?

 

[Darzil]

I think the bug is fixed. I'm not at home for a few days, and have currently only got my dev setup on my desktop. Plan to add it to the laptop tomorrow. I do think that it would be better to rename some of those functions, but clearly as we only use them internally it's not a big deal.

 

[Veracity]

Right. Code clarity is important - and making it less likely that we'll introduce bugs is important - but that can wait.

I'll mark this fixed. If you want to rename functions, by and by, please do.