[1.18.2] Player.hurt() doesn't work

[Koolade446]

So I'm trying to add a piece to my mod to make Stonecutters hurt players. So im listening for WorldTickEvent and if the player is standing on a stone cutter im using Player.hurt(DamageSource.OUT_OF_WORLD, 0.4f)

 

Heres my code:

@Mod.EventBusSubscriber(bus = Mod.EventBusSubscriber.Bus.FORGE)
public class EventListener {

    @SubscribeEvent
    public static void checkStance(TickEvent.WorldTickEvent e) {
        if (Minecraft.getInstance().player != null) {
            if (Minecraft.getInstance().player.level.getBlockState(Minecraft.getInstance().player.getOnPos()).getBlock() instanceof StonecutterBlock) {
                HyperIon.getLOGGER().info("Player is on saw");
                Minecraft.getInstance().player.hurt(DamageSource.OUT_OF_WORLD, 0.4f);
            }
        }
    }
}

The console prints out "player is on saw" whenever I stand on a saw but it just doesn't hurt me and I can't figure out why. 

[MFMods]

impressive, every row in that class is wrong.

this thing Minecraft.getInstance().player - it returns a current player in a client window. it will crash the server. just never use it. never, ever. any damage happens on the server. client is only for showing things to humans.

first of all, pick a better event. try BlockEvent.FarmlandTrampleEvent.

next how to get a player? this is a multiplayer game, you can't say "game, give me the player". the player is in the event (if you are using FarmlandTrampleEvent or LivingUpdateEvent); you will always take player and world/level from event.

next, before hurting a player or spawning a duck or whatever, you need to check whether the world (get it from the player) is client world (player.getLevel().isClientSide()). only if it is not, proceed.

void damage is an odd choice and will give a weird message, but whatever.

finally, consider a cooldown. these tick events fire 20x per second (even after you check client side and in some cases event phase). there is no need to do things 20x per secon, humans can not follow that. try adding   if player.tickCount % 20 == 13. tick count is player's age (sort of); the check i wrote will see that you do damage once per second.

[Koolade446]

13 hours ago, MFMods said:

impressive, every row in that class is wrong.

this thing Minecraft.getInstance().player - it returns a current player in a client window. it will crash the server. just never use it. never, ever. any damage happens on the server. client is only for showing things to humans.

first of all, pick a better event. try BlockEvent.FarmlandTrampleEvent.

next how to get a player? this is a multiplayer game, you can't say "game, give me the player". the player is in the event (if you are using FarmlandTrampleEvent or LivingUpdateEvent); you will always take player and world/level from event.

next, before hurting a player or spawning a duck or whatever, you need to check whether the world (get it from the player) is client world (player.getLevel().isClientSide()). only if it is not, proceed.

void damage is an odd choice and will give a weird message, but whatever.

finally, consider a cooldown. these tick events fire 20x per second (even after you check client side and in some cases event phase). there is no need to do things 20x per secon, humans can not follow that. try adding   if player.tickCount % 20 == 13. tick count is player's age (sort of); the check i wrote will see that you do damage once per second.

Ok, thanks. I haven't ever really done anything server-side mostly client side until mods and mostly 1.8 so sorry for my lack of knowledge of both server-side behavior and current forge events. Thanks for that so Possibly use PlayerTickEvent? How does LivingUpdateEvent fire is it just a tick thing or does the player have to do some kind of action? thanks for your help.

 

 

Edit: Ok heres my now working code
 

@Mod.EventBusSubscriber(bus = Mod.EventBusSubscriber.Bus.FORGE)
public class EventListener {

    @SubscribeEvent
    public static void checkStance(TickEvent.PlayerTickEvent e) {
        if (e.player != null && e.player.level != null) {
            if (e.player.level.getBlockState(e.player.getOnPos()).getBlock() instanceof StonecutterBlock) {
                HyperIon.getLOGGER().info("Player is on saw");
                e.player.hurt(DamageSource.OUT_OF_WORLD, 1.5f);
            }
        }
    }
}

 

Any other advice? I plan to eventually implement a custom DamageSource btw the OUT_OF_WORLD source was just for testing

Edited March 27, 2022 by Koolade446

[Koolade446]

19 hours ago, MFMods said:

if player.tickCount % 20 == 13

the issue with this is if a player gets on a saw and player.tickCount % 20 is 14 the player could have 19 ticks or almost a whole second to get off the saw before a tick of damage would be delt so if they like ran across it they may not take damage. Would something like this work instead:

 

@Mod.EventBusSubscriber(bus = Mod.EventBusSubscriber.Bus.FORGE)
public class EventListener {

    @SubscribeEvent
    public static void checkStance(TickEvent.PlayerTickEvent e) {
        if (e.player != null) {  //I realised checking if the level was null was a mute point because if the player wasn't null then the level couldn't be either
            final int tickRem = e.player.tickCount % 20;
            if (e.player.level.getBlockState(e.player.getOnPos()).getBlock() instanceof StonecutterBlock && e.player.tickCount % 20 == tickRem) {
                e.player.hurt(DamageSource.OUT_OF_WORLD, 30f);
            }
        }
    }
}

To make sure the player always takes damage on the first tick they stand on the saw for or am I misunderstanding something about how ticks work.

Edited March 28, 2022 by Koolade446

[MFMods]

i think you are.       e.player.tickCount % 20 == tickRem   will always be true.

it's simple - every 50ms, tick events are fired for every dimension/world and for every entity and for every block entity (tile entity). there's more to it, but let's keep it simple.

your handler executes within a single tick. tickCount is player's age in ticks. worlds have age too and in tick event you almost always check age modulo and do your thing every second or every five seconds. people mostly write   age % 20 == 0  but i never put zero in those checks; 5 or 13 gives the same effect (frequency of 1 per sec) but doesn't clump a bunch of actions in the same tick.

do not check for player being null in PlayerTickEvent that makes no sense.
you need to check the event phase.
usually you need to check whether player is not in client world but not for this purpose.

[Alpvax]

7 hours ago, Koolade446 said:

the issue with this is if a player gets on a saw and player.tickCount % 20 is 14 the player could have 19 ticks or almost a whole second to get off the saw before a tick of damage would be delt so if they like ran across it they may not take damage.

There are 2 approaches to fix that: either increase the frequency and reduce the damage (e.g. % 10 do 0.75 damage) so that they have less chance to escape (but don't die instantly).

Alternatively, perform the check every tick, but save whether or not the player was on the blade the previous tick, and if not, start damaging once/sec. This approach requires you to save the tick that the player stepped on the blade however (and check that `(current tick - start tick) % 20 = 0`) as well as whether or not the player was previously on the blade. I would suggest using capabilities (or at the minimum an NBT tag containing your modid) for this approach

[Koolade446]

8 hours ago, MFMods said:

e.player.tickCount % 20 == tickRem   will always be true.

I’m facepalming so hard at myself right now cause I was totally about to tell you how you were wrong and then I realized oh tickRem would get refreshed every tick. So then my thought is to use while loops however I tend to try and avoid while loops because they lock up threads. Here’s a major question I have about forge. Is a separate instance of each class instantiated for each player or is it more like spigot where I need to make sure my code is accounting for multiple players? Because my next thought for a solution here would be to use an instance variable or a HashMap with player name or uuid:tickRem that clears as soon as they step off the saw. Also I tested the code doing damage every tick and it works fine but I am worried about system resources on a server with many people running checks every 1/20 of a second could get be too much on a server with many people. 

[Koolade446]

8 hours ago, MFMods said:

do not check for player being null in PlayerTickEvent that makes no sense.

Yeah, see I had the same thought but then I tried to load a single-player world and the game crashed on the Joining World screen with Null Pointer. I think there's a split second where the player hasn't technically logged on but is still ticking idk its weird.