Don't spam player if he did break a block with a plant item in hand (probably harvest)

[benjajaja]

Whe you harvest you get an inmense spam of plant growth information, because you usually break crops with your hands which will fill with the first drop.

We should make it so if you hit a block with an item to get information, it should be cancelled if you did break a block in the process. It's not useful anyway, because to the player it isn't clear if the info gotten is of the broken block or the block above (I think that's how it is now) or whatever.

This should also reduce processing a tiny bit since RB won't have to compute growth conditions on each crop break.

[WildWeazel]

why don't we only process it if it can be planted in the struck block? (dirt/soil/sand/etc)

[ProgrammerDan]

It's a good idea but then we have to test if it can be planted first, which is an additional lookup/test. Probably static neutral in terms of overall processing improvement, or net gain (worsening) since we'd be doing the additional test PLUS all the existing tests if it passes the first test.

[sepiatonal]

I would love to have this in the game. I might even code it later tonight. That spam is annoying and I'm always afraid somebody will message me while I'm harvesting and I'll miss the message.

[benjajaja]

@WildWeazel great idea! @ProgrammerDan right now it's way worse than that, it looks up item in hand, config from table, lightlevel (from another block if needed), biome, surrounding glowstone... I think we can get away with checking if block id from event is one of dirt, grass, farmland, sand or soulsand. That wouldn't even be looking up blocks, just testing the event's block. @soerxpso feel free :+1:

[ProgrammerDan]

@gipsy-king Gross. Agreed, then :)

[sepiatonal]

There's a problem with this actually. When you break a crop, you hit the soil under it unless you click really fast. So I've got a fix that works for sugarcane, cactus, etc. - things where the block you're breaking is at eye level or isn't directly on top of another block - but it doesn't work for potatoes, wheat, etc. and unless we remove the output when you hit dirt it really can't be fixed.

[WildWeazel]

bummer. another option could be to limit messages to once a second or so, but that's a lot of extra work to manage a map of timers

[sepiatonal]

The only other solution I can think of is to disable the messages for a second after a player harvests a crop, and that wouldn't be good at all for server resources. A HashMap would have to be accessed every time anybody breaks any block.

[ProgrammerDan]

It actually wouldn't be bad at all. At most you'd be keeping a decrement list of size 250-300; not a big deal.

My suggestion: thread-safe HashMap with Player UUID as Key and timestamp as value.

Psuedo code:

static ThreadsafeMap<UUID, Long> suppression = new ThreadsafeMap<UUID, Long>();

void onEvent(player) {
     Long allowed = suppression.get(player.getUUID());
     long now = System.currentTimeMillis();
     if (allowed == null || allowed <= now) {
         // do test ...
         suppression.put(player.getUUID(), now + 1000L);
     }
}

resource use is super low, lookups are ~O(1), puts are ~O(1), tests are effectively free ... should be good to go.

Edit: Fixed the bug here too.

[sepiatonal]

Alright, I'll get that done later today.

[ProgrammerDan]

just a comment on my pseudo-code, one modification to the end condition:

replace

 if (allowed == null || allowed <= now) {
     // do test ...
 }
 suppression.put(player.getUUID(), now + 1000L);

with

 if (allowed == null || allowed <= now) {
     // do test ...
     suppression.put(player.getUUID(), now + 1000L);
 }

b/c you definitely don't want to update the suppression timer if the event was suppressed.

[WildWeazel]

I see Dan is less lazy than me

[ProgrammerDan]

Got'cher back, man :)

[sepiatonal]

Alright, should be good. Output is disabled for 1 second after harvesting a crop, using a ConcurrentHashMap as per ProgrammerDan's advice. There's no more spam while harvesting.

[ttk2]

nice, is this good to merge?