Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: DeregisterEventHandler and RemoveListener do not work #92

Closed
l1monenko opened this issue Nov 15, 2023 · 18 comments
Closed

Bug: DeregisterEventHandler and RemoveListener do not work #92

l1monenko opened this issue Nov 15, 2023 · 18 comments
Labels
needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity

Comments

@l1monenko
Copy link

After using these functions, the callbacks still work.

@KillStr3aK
Copy link
Contributor

image

I'll just leave this here

@roflmuffin roflmuffin added bug Something isn't working requires-confirmatiom labels Nov 15, 2023
@l1monenko
Copy link
Author

I tried to use DeregisterEventHandler in the following way, first I register the event RegisterEventHandler<EventRoundEnd>(OnRoundEnd), then in the callback OnRoundEnd after its call DeregisterEventHandler("round_end", OnRoundEnd, false), but the callback OnRoundEnd is still called after the end of the round.

@roflmuffin
Copy link
Owner

I tried to use DeregisterEventHandler in the following way, first I register the event RegisterEventHandler<EventRoundEnd>(OnRoundEnd), then in the callback OnRoundEnd after its call DeregisterEventHandler("round_end", OnRoundEnd, false), but the callback OnRoundEnd is still called after the end of the round.

Hookmode is post by default, so I believe it should be: DeregisterEventHandler("round_end", OnRoundEnd, true)

@l1monenko
Copy link
Author

I tried to use DeregisterEventHandler in the following way, first I register the event RegisterEventHandler<EventRoundEnd>(OnRoundEnd), then in the callback OnRoundEnd after its call DeregisterEventHandler("round_end", OnRoundEnd, false), but the callback OnRoundEnd is still called after the end of the round.

Hookmode is post by default, so I believe it should be: DeregisterEventHandler("round_end", OnRoundEnd, true)

I tried DeregisterEventHandler("round_end", OnRoundEnd, true) but also without result.

@l1monenko
Copy link
Author

@roflmuffin Is it still an issue?

@roflmuffin
Copy link
Owner

roflmuffin commented Feb 22, 2024

As long as you use the same function reference that was originally used when you called register (and with the same post bool) then yes, it should work. If you provide a basic repro example then I'll be happy to look into it.

@roflmuffin roflmuffin added needs-author-action An issue or pull request that requires more info or actions from the author. and removed bug Something isn't working labels Feb 22, 2024
Copy link

This issue has been marked needs-author-action and may be missing some important information.

Copy link

github-actions bot commented Mar 8, 2024

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@ssypchenko
Copy link

ssypchenko commented Mar 17, 2024

Here is the basic test confirming that DeregisterEventHandler does not deregister event:

using CounterStrikeSharp.API;
using CounterStrikeSharp.API.Core;
using CounterStrikeSharp.API.Core.Plugin;
using CounterStrikeSharp.API.Core.Attributes.Registration;
using CounterStrikeSharp.API.Modules.Commands;
using CounterStrikeSharp.API.Modules.Utils;
namespace TestPlugin
{
    public class TestPlugin : BasePlugin
    {   
        public override string ModuleName => "Test Plugin";
        public override string ModuleVersion => "v1.0.0";
        public override void Load(bool hotReload)
        {
            Console.WriteLine($"Test started");
        }
        public override void Unload(bool hotReload)
        {
            //
        }
        [ConsoleCommand("teston", "Test")]
        public void OnTestCommand(CCSPlayerController? pc, CommandInfo command)
        {
            Console.WriteLine("Set PlayerSpawn on");
            RegisterEventHandler<EventPlayerSpawn>(OnPlayerSpawnHandler);
        }
        [ConsoleCommand("testoff", "Test")]
        public void OnTestOffCommand(CCSPlayerController? pc, CommandInfo command)
        {
            Console.WriteLine("Set PlayerSpawn off");
            DeregisterEventHandler("player_spawn", OnPlayerSpawnHandler, true);
        }
        private HookResult OnPlayerSpawnHandler (EventPlayerSpawn @event, GameEventInfo info)
        {
            var playerController = @event.Userid; 
            if (playerController == null || !playerController.IsValid || playerController.IsHLTV) {
                return HookResult.Continue;
            }
            Server.PrintToChatAll("Registered");
            return HookResult.Continue;
        }

There is no "Registered" in the chat on plugin start.
There is "Registered" in the chat on every respawn after the command "teston"
There is "Registered" in the chat on every respawn after the command "testoff" (which should deregister the handler)

@ssypchenko
Copy link

@roflmuffin I know you have a lot to do. So this is just a gentle reminder that you "'ll be happy to look into it."

@roflmuffin
Copy link
Owner

I did have an initial look using the repro code you supplied, and was able to replicate it, but also not entirely sure why its happening. We use a dictionary to store these handlers and then it removes the handler based on the dictionary look up, but for some reason the identity is different, perhaps due to the untyped nature of DeregisterEventHandler vs RegisterEventHandler<T>

@B3none
Copy link
Collaborator

B3none commented Mar 26, 2024

Here's a working version of me registering and de-registering an OnTick listener:
https://github.com/roflmuffin/CounterStrikeSharp/blob/main/managed/CounterStrikeSharp.API/Modules/Menu/CenterHtmlMenu.cs#L65-L129

@ssypchenko
Copy link

ssypchenko commented Mar 27, 2024

Here's a working version of me registering and de-registering an OnTick listener:

@B3none
Listeners are registered and removed correctly. Events handlers are not :(

@roflmuffin
Do you mean in your explanations that something is wrong with the syntaxis?
I tried in this way:

RegisterEventHandler<EventPlayerSpawn>(OnPlayerSpawnHandler);
private HookResult OnPlayerSpawnHandler<T>(T @event, GameEventInfo info) where T : GameEvent
{
//code
}
DeregisterEventHandler("player_spawn", OnPlayerSpawnHandler<EventPlayerSpawn>, true);

The result is the same, the event is still active :(
Could you please help me to find a way how to do it correctly?
It may be convenient to deregister events for some reasons. Now I have to use global variable and hookresult.continue when it false. But this is not clean :(

@roflmuffin
Copy link
Owner

Here's a working version of me registering and de-registering an OnTick listener:

@B3none Listeners are registered and removed correctly. Events handlers are not :(

@roflmuffin Do you mean in your explanations that something is wrong with the syntaxis? I tried in this way:

RegisterEventHandler<EventPlayerSpawn>(OnPlayerSpawnHandler);
private HookResult OnPlayerSpawnHandler<T>(T @event, GameEventInfo info) where T : GameEvent
{
//code
}
DeregisterEventHandler("player_spawn", OnPlayerSpawnHandler<EventPlayerSpawn>, true);

The result is the same, the event is still active :( Could you please help me to find a way how to do it correctly? It may be convenient to deregister events for some reasons. Now I have to use global variable and hookresult.continue when it false. But this is not clean :(

No there is nothing wrong with your syntax, I believe it is an issue on our end since the register requires a strongly typed T type event handler but the deregister event handler accepts any Delegate

Copy link

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

@roflmuffin
Copy link
Owner

There is now a DeregisterEventHandler<T> which should fix this problem if you would be able to try that. It should be in v208.

@ssypchenko
Copy link

There is now a DeregisterEventHandler<T> which should fix this problem if you would be able to try that. It should be in v208.

Yes, it works Ok now. Thank you!

Copy link

This issue has been automatically marked no-recent-activity because it has not had any activity for 14 days. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will remove no-recent-activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-author-action An issue or pull request that requires more info or actions from the author. no-recent-activity
Projects
None yet
Development

No branches or pull requests

5 participants