Feature - Implemented Working on a somewhat large change for tracking attributes and quality of effects

gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
I'd like to be able to know if KoL considers an effect good, neutral or bad, and know flags like not pvp decrementing, no hookah, no remove etc etc. I've coded some preliminary support for this along with spaded data (thanks to Aenimus, The Dictator etc and others in the spading channel on the ASS Discord)

Just for reviewing/sense-checking as I go, I'm posting an unfinished patch, although this builds and works for me at the moment.

One thing I added was some logging for Genie wishes denying you a wish that we think it should allow, or visa versa (similar to how mafia warns you that it thinks something is multiusable when the data says otherwise and visaversa). It might also be nice to add this for an effect coming from the hookah, crazy horse etc.

While I'm thinking about it, it might similarly be interesting to add some output when copying a monster fails that we know to be copyable or visa versa. And checking other flags too!
 

Attachments

  • quality_and_attrs_patch.zip
    175.4 KB · Views: 21
fronobulax

fronobulax

Developer
Staff member
I'm not quite understanding what problem you are trying to solve or capability you are trying to add.
 
Aenimus

Aenimus

Member
Knowing which effects can be received from pizzas? Or wished for? These are two big ones, among other benefits. I'm not sure what you don't understand, fronobulax.
 
gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
I don't mean to be rude, but my first sentence is literally describing what I'd like to be able to do with mafia that I can't currently.
 
fronobulax

fronobulax

Developer
Staff member
Aenimus said:
Knowing which effects can be received from pizzas? Or wished for? These are two big ones, among other benefits. I'm not sure what you don't understand, fronobulax.
Click to expand...

What I don't understand is the mechanics, in both KoL and in mafia, of items I don't have, or don't use, or don't understand the difficulties associated with scripting for them. So I don't recognize a general description of a solution to a problem that I am not aware of.

Including "pizza" in the first post would have pointed me in the right direction.

Thanks.
 
Veracity

Veracity

Developer
Staff member
Comment:

You are changing the format of statuseffects.txt. You need to increase the version number (first line of the file) from 3 to 4 and similarly bump KoLConstants.STATUSEFFECTS_VERSION.
You changed the formatting of a lot of existing lines. For example, from the .patch file:

Code: 
-	public static final void writeEffects( final File output )
-	{
-		RequestLogger.printLine( "Writing data override: " + output );
-		PrintStream writer = LogStream.openStream( output, true );
-		writer.println( KoLConstants.STATUSEFFECTS_VERSION );
+	public static final void writeEffects(final File output) {
+		RequestLogger.printLine("Writing data override: " + output);
+		PrintStream writer = LogStream.openStream(output, true);
+		writer.println(KoLConstants.STATUSEFFECTS_VERSION);
KoLmafia's standard is to have spaces after "(" and before ")".

Code: 
-			for ( int i = lastInteger; i < nextInteger.intValue(); ++i )
-			{
-				writer.println( i );
+			for (int i = lastInteger; i < nextInteger.intValue(); ++i) {
+				writer.println(i);
KoLmafia coding style does not "cuddle" braces.

Code: 
-	public static final AdventureResult getFirstMatchingEffect( final String parameters )
-	{
-		return EffectDatabase.getFirstMatchingEffect( parameters, true );
+	public static final AdventureResult getFirstMatchingEffect(final String parameters) {
+		return EffectDatabase.getFirstMatchingEffect(parameters, true);
Especially not on method definitions!

Code: 
-	public static final int[] POISON_ID = {
-   		0,
-   		EffectPool.TOAD_IN_THE_HOLE,
-   		EffectPool.MAJORLY_POISONED,
-   		EffectPool.REALLY_QUITE_POISONED,
-   		EffectPool.SOMEWHAT_POISONED,
-   		EffectPool.A_LITTLE_BIT_POISONED,
-   		EffectPool.HARDLY_POISONED
-   	};
+	public static final int[] POISON_ID = { 0, EffectPool.TOAD_IN_THE_HOLE, EffectPool.MAJORLY_POISONED,
+			EffectPool.REALLY_QUITE_POISONED, EffectPool.SOMEWHAT_POISONED, EffectPool.A_LITTLE_BIT_POISONED,
+			EffectPool.HARDLY_POISONED };
One line per initialized element.

Your patch would be a lot smaller - and it would be a lot easier to understand what actually changed - if you did not have so many "changes" that are simply changing the formatting away from the "standard" style it was already in.

Can you restore the changes that are simply formatting changes back to what they were before, please?

Thanks.
 
T

Terrabull

Member
This seems like it might be in the same scope as your project, so what about monsters that can't be sniffed/forced/arrowed/duplicated, etc. Also, some monsters have a soft block to an item (preventing you from using it.) Not sure if that's something you'd like to track.
 
gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
Veracity said:
Comment:

You are changing the format of statuseffects.txt. You need to increase the version number (first line of the file) from 3 to 4 and similarly bump KoLConstants.STATUSEFFECTS_VERSION.
You changed the formatting of a lot of existing lines. For example, from the .patch file:

...
Click to expand...

Yes of course! I didn't do those on purpose and I'm aware of the standard. My editor occasionally decides to jump in and make "helpful" changes but I didn't realise it had done it at this scale. I didn't actually read through the patch, foolishly.
 
Last edited:
gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
Autoformatter is now off and all those erroneous changes are cleaned up
 

Attachments

  • quality_and_attrs.zip
    170.5 KB · Views: 16
Veracity

Veracity

Developer
Staff member
That was a lot of work!

Comments:

We have Java 8 now. So, you can use the Java 7 diamond operator. I will be using that henceforth.

For example, your new code:

Code: 
+	private static final Map<Integer, Integer> qualityById = new HashMap<Integer, Integer>();
+	private static final Map<Integer, List<String>> attributesById = new HashMap<Integer, List<String>>();
could be:

Code: 
+	private static final Map<Integer, Integer> qualityById = new HashMap<>();
+	private static final Map<Integer, List<String>> attributesById = new HashMap<>();
The following cannot be right:

Code: 
-		if ( descriptionId != null )
+		if ( descriptionId != "" )
Perhaps you want:

Code: 
-		if ( descriptionId != null )
+		if ( descriptionId != null && !descriptionId.equals( "" ) )
I see that Java 7 allows strings as case labels in switch statements - which you use. But I don't think it makes == the same as .equals() for Strings. Am I wrong?

Other than those minor things, this looks good - and pretty interesting. Get it in sooner, rather than later, in my opinion, before we start getting new effects to add, since, near as I can tell, you are generating the correct new statuseffects.txt format when we register new effects. (You did test with "test neweffect 4bc3e6784775746fae40f1f019dd0ec0", for example?)
 
gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
r19820 and r19821!
 
ckb

ckb

Minion
Staff member
I believe this will be very useful, but I would like to know more about how effects are defined.

quality is: good, neutral, bad - what do each of those mean for an effect?

attributes: blank or nohookah - I assume "nohookah" means the effect cannot be gained from the $item[ittah bittah hookah]. Does this have other implications (from Genie wishes or diabolic pizza effects?)

There is now 1 effect with attribute "none" - $effect[Warmed Up]. Is this intentional?

Thanks.
 
Aenimus

Aenimus

Member
So, "nohookah" is a tag devs have used (in public) for a category of effects. It is the name of the tag on /dev. It originally meant it could not be received from the hookah (the only and original source), but now extends to other things. An example of an effect tagged "good" and "noohokah" is Jukebox Hero.

Examples of "hookah-like" things that follow similar rules: ittah bittah hookah, Glenn's golden dice, glob of undifferentiated tissue, crazy horse, yummy tummy bean(?)

Hookah-like stuff can only roll "good" effects. The good category is usually defined by a net positive result; e.g., +10% meat or +50% stats, or a combination of positive stuff. This is why you cannot get avatar effects, because they are presumably "neutral". Combat modifiers are also neutral (since they aren't always good), and maybe ML as well; I am not sure.

You can get neutral effects from diabolic pizzas and wishing, however. Avatar potions are specifically blocked from pizzas (because that would suck). You cannot get "bad" effects from the pizza (but maybe you can wish for them?).

It's possible that you can get certain effects from one source but cannot wish for them. I don't have an example.
 
gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
I originally intended them all to have "none" to make the sequence of nullable fields in a tab-separated data files more clear, and the fact that they're blank is an... unintended feature. Maybe I should remove "none" from the new effect registration.
 
Veracity

Veracity

Developer
Staff member
You should make sure that what you print agrees with what you want to be in statuseffects.txt.

1) If you think that "none" is more clear than an empty field with a tab on both sides (and it is), then you should update statuseffects.txt to have that, agreeing with new effect registration.
2) If that is too tedious or difficult, you should fix new effect registration to not print none and remove none from the handful of new effects added since this feature went in.

I think having "none" is better, so I vote for option 1.
But you need to do either 1 or 2. And then mark this feature Implemented.
 
S

Saklad5

Member
I remember opening a request for tracking PVP-decrementing ages ago. Glad to see it being implemented by someone.


I can sympathize with auto-formatting woes. It’s one of the reasons I use Vim: it is fairly easy to make it use existing formats over defaults if they exist. Still, it’s always worth quickly checking patch files (or git commits, or whatever is being used) to make sure every changed line is actually relevant.
 
gausie

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
Veracity said:
You should make sure that what you print agrees with what you want to be in statuseffects.txt.

1) If you think that "none" is more clear than an empty field with a tab on both sides (and it is), then you should update statuseffects.txt to have that, agreeing with new effect registration.
2) If that is too tedious or difficult, you should fix new effect registration to not print none and remove none from the handful of new effects added since this feature went in.

I think having "none" is better, so I vote for option 1.
But you need to do either 1 or 2. And then mark this feature Implemented.
Click to expand...

I will do this asap, and implement your preferred option of course. Little snowed under at the moment unfortunately.
 
You must log in or register to reply here.
Top