kernitus / BukkitOldCombatMechanics

Spigot plugin to configure combat mechanics for 1.9 onwards
https://www.spigotmc.org/resources/19510/
Mozilla Public License 2.0
178 stars 71 forks source link

OCM 1.6.6 and AnimatedNames #217

Closed AmazedMender16 closed 6 years ago

AmazedMender16 commented 6 years ago

Problem Description

Using OCM to disable play collisions per world, Tho once you go to the world where its disabled the AnimatedNames disapears.

rayzr522 commented 6 years ago

So if I'm understanding correctly, AnimatedNames works fine with OCM if the disable player collisions module is allowed in that world, but as soon as you move to a world that doesn't have player collisions disabled, the AnimatedNames disappear?

Also, a link to the plugin page for AnimatedNames would be appreciated. I'm not entirely sure which plugin you're using.

AmazedMender16 commented 6 years ago

Hello,

Yeah once we disable player-collisions inside the config and go to that world they wont dissapear. Once we enable it (so player collisions is disabled in that world) and go to that world the TAG dissapears untill you move out of the world and relog.

AnimatedNames: https://www.spigotmc.org/resources/animatednames.2175/

It is working fine with a custom version that a user gave me a few weeks back, which was posted here: https://github.com/gvlfm78/BukkitOldCombatMechanics/issues/173.

rayzr522 commented 6 years ago

So it works fine in the version that @I-Al-Istannen sent you, but not in 1.6.6?

AmazedMender16 commented 6 years ago

Correct, with 1.6.6 the name disappears when entering a world that has player collisions disabled.

I-Al-Istannen commented 6 years ago

I suspect it is due to this code, which was introduced in Make the collision per-world:

    @EventHandler(priority = EventPriority.MONITOR)
    public void onPlayerChangeWorld(PlayerChangedWorldEvent e){
        if(isEnabled(e.getPlayer().getWorld())){
            TeamUtils.sendTeamPacket(e.getPlayer());
        } else {
            TeamUtils.sendTeamRemovePacket(e.getPlayer());
        }
    }

AnimatedNames likely doesn't reinstate its scoreboard.

Possible mitigations

Do not send a team disband packet

This will prevent this problem, but you will carry over the collision state into worlds that have collision enabled (i.e. the module disabled). This means that if you have two worlds, WC (with collisions) and WOC (without collisions) and go from WOC to WC you will still have no collisions.

Send a team disband packet and magically find out if another plugin relied on the team

Maybe do that by checking any team packet send and finding out if you just killed it. In that case, reinstate it.

Make it a setting in the config to disable that and make the Packet Listener toggle the collision flag

This way collision will be working as long as you have a plugin always sending team packets (Which AnimatedNames seems to do)

Anything I missed?


What would you prefer, Rayzr and gvlfm78?

kernitus commented 6 years ago

@I-Al-Istannen Option 3 seems to be the best right now, unless the "magic" involved in option 2 is in fact very simple.

I-Al-Istannen commented 6 years ago

Sorry for taking so long, I was on holiday :)

I have no way to test anything I do, so I guess I will throw it in a config option and see if that helps. There isn't much I can do without knowing what the other plugin actually does and being able to test a bit here and there, so I guess I will go with the most coarse setting.

Please add the config option as shown in the screenshot:

enableTeamDisbanding: false

grafik

And here is a test build: OCM.zip

@AmazedMender16

AmazedMender16 commented 6 years ago

Hello @I-Al-Istannen

I will give it a shot!

Regards, Glenn R.

AmazedMender16 commented 6 years ago

Hello,

It is working, the names are not dissapearing. Tho the ColorCodes are messed up.. Their names should be white instead of blue. http://prntscr.com/kqixpk

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

And I suppose it works without OCM's collisions module enabled?

It's something though, but this error is even stranger than the last.

AmazedMender16 commented 6 years ago

Hello,

This is what we are using.

disable-player-collisions:
  # This is to disable player collision
  # This is now compatible with scoreboard and tablist-editing plugins
  enabled: true
  worlds: [W2]
  enableTeamDisbanding: false

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Hey, I meant this: Are the names white when you are not using OCM's collisions? I.e. if you set enabled to false.

AmazedMender16 commented 6 years ago

Hello,

Sorry false alarm.... Its because we were using 1.13.1 clients...

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

No worries :) So the fix works?

I'd really appreciate if you could verify that collision is disabled when moving from a collision world to a non-collision one. If that works, I will PR this change into the master branch, so it is hopefully available in the next release.

Thank you for trying the weird things I suggested so far ;)

AmazedMender16 commented 6 years ago

Hello,

The names yeah... Tho I just found out that the collisions don't work no more.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

That is what I feared...

You re-enabled the module (enabled), yea?

Do you mean that no world has collisions anymore or that all worlds have them?

AmazedMender16 commented 6 years ago

Hello,

It doesn't work in general, yeah i re-enabled the module :)

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Works for me in 1.13, but only when the collisions module was enabled when I joined. Can you both leave and rejoin when it is enabled and see what happens? If it persists, it was this change that broke it.

AmazedMender16 commented 6 years ago

Hello @I-Al-Istannen

Servers are running on 1.12.2 and have 1.13.1 client support. I will give rejoining a shot, even tho I restarted the server.

Edit: Still can't walk thru players in any worlds.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Hm. I hope the protocol hack doesn't change things, but it should theoretically be disabled if you join with an 1.12.2 client.

The change I made has nothing to do with not being able to walk through players though, it makes the opposite possible in disabled worlds, so it might not be this change we are testing.

If there are no errors in the chat, what is your protocol hack called?

AmazedMender16 commented 6 years ago

Hello,

We are using viaversion 1.5.1-SNAPSHOT Also seems like the 1.8 PVP check has been enabled while its disabled.

Regards, Glenn R.

AmazedMender16 commented 6 years ago

Hello,

Guess what.... It created a new config.... Editing the new config and the names are gone again... Tho the collisions are working the way they should.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Even with the new option (enableTeamDisbanding) set to false?

If that is the case I also don't get why the old one worked, but I do not want to burden you to test a slightly different version every few minutes. I will see if I can get a version of AnimatedNames from the dev or ask him what exactly his plugin sends, because debugging it becomes annoying for both of us otherwise.

AmazedMender16 commented 6 years ago

Hello,

Yeah enableTeamDisbanding is set to false. I hope he will give you a copy to test with.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Hey,

that is sad. I will send him a message, but that makes an ETA a bit harder :/

Thanks for your patience!

I-Al-Istannen commented 6 years ago

@AmazedMender16 Just to get the current status into a more expressive form, could you make a short video/gif of the problem? From what I gathered that'd be changing the world and making a video/gif of the name tag disappearing?

Thank you :)

AmazedMender16 commented 6 years ago

Hello,

I'm not in a situation that I can do that... just, when you change worlds the prefix disappears. http://prntscr.com/kqkww1 It will just show AmazedMender16 in white letters after moving worlds.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Thanks, that should do as well. I had some trouble correctly restating the gist of it as some things changed (colour wrong ==> 1.13 via-version "problem"), etc.

But that should be fine. Thank you!

AmazedMender16 commented 6 years ago

Hello,

The color issues are only there when using 1.13.1 when using 1.12.2 the colors are correct. The prefix is disappearing when using 1.12.2 aswell.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

MvdW said that the color is normal:

ViaVersion does not correctly change the behavior of scoreboards on 1.13.1 clients 1.12.x -> Player name = last color of the prefix 1.13.x -> Player name = Team color ViaVersion does not manually set the team color to the prefix's last color (for 1.13.1) clients causing it > to cause color problems.

Which is why I said that it is a ViaVersion/1.13 "problem". Seems to be just a subtle change in the vanilla client and not really a bug.

The disappearing with the version I sent in this thread and enableTeamDisbanding set to false is the bug (if any of the two are not correct, i.e. you are not using the version I sent here or enableTeamDisbanding is true/not set please say so). I have relayed this information to MvdW and we will seen if (h/w)w can work something out.

AmazedMender16 commented 6 years ago

Hello,

Thats what I said the colors are fine, just the prefix is disappearing.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Hey, can you try this release when you have time?

The enableTeamDisbanding still needs to be false.

AmazedMender16 commented 6 years ago

Hello,

Name seems to be fixed... Will report back on that later aswell. Tho the per world funtion in broken now.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Can you try this one then? He didn't give me a copy, but he made me realize some other error. If this works, it would be nice. If not I will have to get a bit more creative, but at least I found the cause for the names.

Sorry for having so many versions to test through, but the last two actually cleared up the cause. I am just hoping I can get away with the lazy approach I took in this build, otherwise it will be a bit more complicated.

Thank you and have a nice day!

AmazedMender16 commented 6 years ago

Hello,

No problem :) Names are staying, tho now you're able to push players in any world. So disable collisions is not working.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Alright, I changed how collision packets are handled, as the old way was a patchwork of something gvlfm78 found somewhere and some adjustments by me. The new one is a complete rewrite with compatibility in mind. Try this please.

EDIT: Updated the link to fix a bug when a disband was not followed by a world change/create packet. Now the create packet is sent by us.

AmazedMender16 commented 6 years ago

Hello,

Sorry for the late response, will give it a shot when I get home!

Regards, Glenn R.

AmazedMender16 commented 6 years ago

Hello,

Names are staying tho console is being spammed with the following. [17:30:04] [Netty Epoll Server IO #1/INFO]: Collision rule is ALWAYS for action UPDATE [17:30:04] [Netty Epoll Server IO #1/INFO]: Collision rule is NEVER for action UPDATE [17:30:05] [Netty Epoll Server IO #1/INFO]: Collision rule is ALWAYS for action UPDATE [17:30:05] [Netty Epoll Server IO #1/INFO]: Collision rule is ALWAYS for action UPDATE

Also per world collisions isn't working, its now enabled in all worlds.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

Yes, that "spam" is my debug output. It works correctly for me (go figure) with NametagEdit which should employ similiar techniques.

If you go to a world with collision disabled (on a test server with only you/two players if that is possible), does it say "Collision rule is ALWAYS" or "Collision rule is NEVER"? Because in the snippet you showed it deliberately enabled collisions (Collision rule is ALWAYS), i.e. it thought they should be enabled in that world.

Are you sure the world settings are correct? If yes I will add some debug output that logs the enabled worlds so you can see if it really enabled it in the world.

EDIT: You made me realize that this will always enable collisions when the module is disabled, i.e. it still does something in that case. That shouldn't really matter unless another plugin/vanilla scoreboards are used as well with conflicting rules, but I will remove that. Doesn't relate to this problem though.

AmazedMender16 commented 6 years ago

Hello,

Hmmmm I have way more fun Info for you :)

Player collisions is enabled, when you join the server in the world with the collisions enabled you won't be able to push players neither if you move worlds. Tho if you rejoin from the other world you won't be able to push anything not even in the world specified in the config

Hope this makes sense ^^

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

What is the respective output in the console? Does it say you anything in chat?

I-Al-Istannen commented 6 years ago

You can use this build to automatically attach the world name to the logs.

AmazedMender16 commented 6 years ago

https://pastebin.com/L9QSwYmv

I-Al-Istannen commented 6 years ago

Can you try the newer build in my last message, post the new log (which now contains the world name) and post the relevant config section (for this module) as well?

Thank you very much :)

AmazedMender16 commented 6 years ago

Same is happening as I said in my previous post: https://pastebin.com/L9QSwYmv

disable-player-collisions: enabled: true worlds: [GR231] enableTeamDisbanding: false

I-Al-Istannen commented 6 years ago

OCM does not disable collisions between you and other mobs (it never did), are you aware of that? The log looks correct: Collisions are disabled in the GR231 world and enabled in the rest.

That is not the behaviour you are observing?

AmazedMender16 commented 6 years ago

Hello,

I'm not seeing that happen... When i join in GR231 player colissions stays enabled in all worlds. If I join in the end player colissions will stay disabled.

Regards, Glenn R.

I-Al-Istannen commented 6 years ago

All mentions of the GR231 world are collision rule NEVER (so there should be no collisions at all in that world) and the rules for the other worlds are ALWAYS, so you should have collisions there. This works flawlessly on my server, with exactly the same packets send/modified, which is weird.

What happens when you remove AnimatedNames?

AmazedMender16 commented 6 years ago

Same things are happening when animatednames is removed, have to rejoin to disable/enable player collisions per world.

I-Al-Istannen commented 6 years ago

Alright, then your system seems to differ in a crucial part from mine. Server version? Client version? Other plugins that might send packets? Any scoreboard teams set up?