CivClassic / JukeAlert

A plugin that creates a new block in Minecraft that records player entries and actions. Anti-grief tool. Built for Paper 1.16.5
BSD 3-Clause "New" or "Revised" License
0 stars 18 forks source link

Fixed /ctf not creating snitches #22

Closed Falvyu closed 4 years ago

Falvyu commented 4 years ago

Note: Closed #17 and made a new commit because of git shenanigans on my part. I apologize for the trouble.

Fix to #16

The ReinforcementCreationEvent handler was executed before the call to BlockPlaceEvent. This meant that: pendingSnitches.remove(block.getLocation()) in the RCE handler was returning null and prevented the snitch creation.

I got busy IRL shortly the time I made the first PR and I haven't really looked into it after that. Since it seems to still an issue, I'm taking another look into it. (I assume no one is working on it right now)

I've set the priority to NORMAL instead of LOW as indicated.

I think it'd be good to have two listeners here:

  • The current one at NORMAL priority (which would still be before Citadels HIGH)
  • An additional one at MONITOR priority which removes the block from pendingSnitches if the event was cancelled

If I understand correctly there should be 3 listeners instead of two, am I correct ?

Maxopoly commented 4 years ago

Is this PR missing commits? Check the diff, it's only one line.

Maxopoly commented 4 years ago

If I understand correctly there should be 3 listeners instead of two, am I correct ?

Yes, as described by you

Falvyu commented 4 years ago

Is this PR missing commits? Check the diff, it's only one line.

I did mess up locally but yes, only one line was changed. (only the event priority) I've now added the MONITOR handler as requested. It will be called when the BlockPlaceEvent is cancelled by a previous listener and will remove pending snitches in the eponymous set.

It works on my 1.14 test server but I haven't tested it on 1.16 however.

Maxopoly commented 4 years ago

Good to merge, will go in after the 1.16 update