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

[ecs] Potential Duplicate Entity Problem? #448

Open
KoertLichtendonk opened this issue Jan 26, 2025 · 7 comments
Open

[ecs] Potential Duplicate Entity Problem? #448

KoertLichtendonk opened this issue Jan 26, 2025 · 7 comments

Comments

@KoertLichtendonk
Copy link

KoertLichtendonk commented Jan 26, 2025

I assume that this is a rare SampSharp bug that occurs in some weirdly specific situation as I have no other explanation for this. On my server it randomly started sending double messages in chat to every player. Never had this happen before, and haven't done any changes to the chat logic that could've caused it.

Image

The OOC command loops through every Player and does SendClientMessage, so I don't think there can go much wrong on my side. It almost feels like there are two of the same Player entities.

Unfortunately there isn't much else I can tell you to narrow down the potential issue, it randomly happened after I logged in, before that everything was fine. Here is the /ooc code:

    public class ChatCommands : ISystem
    {
        [ServerCommand(PermissionGroups = new string[] { "Default" },
            Description = "Send a message to the global Out-Of-Character (OOC) chat. Use /ooc followed by your message to communicate OOCly with all players.",
            CommandGroups = new[] { "Chat" })]
        public void Ooc(Player player, IEntityManager entityManager, IDiscordService discordService, string text)
        {
            if(player.IsPlayerPlayingAsCharacter())
            {
                player.SendPlayerChatMessage(entityManager, PlayerChatMessageType.OOC, text);
                discordService.SendGlobalOocChatMessage(player, text);
            }
        }
    }

    public static class ChatHelper
    {
        public static void SendPlayerChatMessage(this Player player, IEntityManager entityManager, PlayerChatMessageType type, string text)
        {
            if (player.IsPlayerPlayingAsCharacter())
            {
                CharacterModel character = player.GetPlayerCurrentlyPlayingAsCharacterModel();
                switch (type)
                {
                    case PlayerChatMessageType.OOC:
                        string CHAT_ACTION_OOC = String.Format("(( OOC | {0}: {1} ))", character.GetCharacterName(), text);

                        foreach (Player foreachPlayer in entityManager.GetComponents<Player>())
                        {
                            foreachPlayer.SendClientMessage("{D3D3D3}" + CHAT_ACTION_OOC);
                        }
                        break;
                }
            }
        }
    }

I'm curious if anyone else has ever had this issue?

I have a Discord bot that sends messages from /ooc to Discord, and in Discord only one message appears, which is another reason that leads me to believe this might be a rare SampSharp bug.

@KoertLichtendonk
Copy link
Author

KoertLichtendonk commented Jan 26, 2025

Actually, after we were done testing we both went off and I came back on later and saw the server had restarted/crashed. I went into the log and saw an exception before the crash when my buddy was writing something to me right after I left the server. There were no exceptions when we were both online.

[SampSharp:ERROR] Unhandled exception: System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> SampSharp.Entities.EntityNotFoundException: The specified entity could not be found. (Parameter 'entity')
   at SampSharp.Entities.EntityManager.GetComponent[T](EntityId entity)
   at SampSharp.Entities.Component.GetComponent[T]()
   at OpenRP.GameMode.Features.Chat.Helpers.ChatHelper.SendTalkMessage(Player player, IEntityManager entityManager, String text) in /app/src/OpenRP.GameMode/Features/Chat/Helpers/ChatHelper.cs:line 114
   at lambda_method18(Closure , Object , Object[] , IServiceProvider , IEntityManager )
   at SampSharp.Entities.EventService.<>c__DisplayClass11_0.<CreateInvoker>b__0(Object instance, EventContext eventContext)
   at SampSharp.Entities.EventService.Invoke(EventContext context)
   at SampSharp.Entities.SAMP.EntityMiddleware.Invoke(EventContext context, IEntityManager entityManager)
   at lambda_method43(Closure , Object , EventContext , IServiceProvider )
   at SampSharp.Entities.EcsBuilderUseMiddlewareExtensions.<>c__DisplayClass3_1.<UseMiddleware>b__2(EventContext context)
   at SampSharp.Entities.EventService.<>c__DisplayClass12_0.<BuildInvoke>b__0(Object[] args)
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Span`1& arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at SampSharp.Core.Callbacks.Callback.Invoke(Byte[] buffer, Int32 startIndex)
   at SampSharp.Core.HostedGameModeClient.PublicCall(String name, IntPtr data, Int32 length)

@KoertLichtendonk KoertLichtendonk changed the title [ecs] Potential Duplicate Message Problem? [ecs] Potential Duplicate Entity Problem? Jan 26, 2025
@ikkentim
Copy link
Owner

ikkentim commented Jan 26, 2025

All sampsharp does is splitting messages longer than limit to separate messages. Haven't seen this happen before.

Based on your code it may indicate entityManager.GetComponents<Player>() returns (some) players more than once. This either means something in sampsharp is broken, or maybe a PlayerDisconnectMiddleware isn't executed properly. Maybe an exception has occured in OnPlayerDisconnect?

@KoertLichtendonk
Copy link
Author

KoertLichtendonk commented Jan 27, 2025

Apart from the earlier exception I also found this:

[2025-01-26T19:10:20+0000] [Info] [part] Kir_Polzin has left the server (1:1)
Unhandled exception. SampSharp.Entities.EntityNotFoundException: The specified entity could not be found. (Parameter 'entity')
   at SampSharp.Entities.EntityManager.GetComponent[T](EntityId entity)
   at SampSharp.Entities.Component.GetComponent[T]()
   at OpenRP.GameMode.Features.ChickenCoop.Components.ChickenCoop.UpdateChickenCoop(IEntityManager entityManager) in /app/src/OpenRP.GameMode/Features/ChickenCoop/Components/ChickenCoop.cs:line 74
   at OpenRP.GameMode.Features.ChickenCoop.Services.ChickenCoopManager.RecurringTimer() in /app/src/OpenRP.GameMode/Features/ChickenCoop/Services/ChickenCoopManager.cs:line 28
   at OpenRP.GameMode.Features.ChickenCoop.Services.ChickenCoopManager.<.ctor>b__3_0(Object _) in /app/src/OpenRP.GameMode/Features/ChickenCoop/Services/ChickenCoopManager.cs:line 23
   at System.Threading.TimerQueueTimer.<>c.<.cctor>b__27_0(Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.TimerQueueTimer.CallCallback(Boolean isThreadPool)
   at System.Threading.TimerQueueTimer.Fire(Boolean isThreadPool)
   at System.Threading.TimerQueue.FireNextTimers()
   at System.Threading.TimerQueue.AppDomainTimerCallback(Int32 id)

What's strange about this one is that I'm not even using Player in the ChickenCoopManager and it's certainly not used in any OnPlayerDisconnect.

        public ChickenCoopManager(IEntityManager entityManager, IStreamerService streamerService)
        {
            _entityManager = entityManager;
            _streamerService = streamerService;
            _recurringTimer = new Timer(_ => RecurringTimer(), null, TimeSpan.Zero, TimeSpan.FromMinutes(10)); // Line 23
        }

        private void RecurringTimer()
        {
            foreach (ChickenCoop.Components.ChickenCoop chickenCoop in GetAllChickenCoops()) // Line 28
            {
                chickenCoop.UpdateChickenCoop(_entityManager);
            }
        }

EDIT: Player is used in the ChickenCoop component, though:

        public void UpdateChickenCoop(IEntityManager entityManager)
        {
            if (DateTime.Now > _nextChickenBred)
            {
                _chickens++;
                _nextChickenBred = DateTime.Now.AddHours(1);
                foreach (Player player in entityManager.GetComponents<Player>())
                {
                    player.PlayOpenCdnStream("sfx", "chickenActivity.mp3", _objectLinkedTo.Position, 3.0f);
                }
            }
            if (DateTime.Now > _nextEggLaid)
            {
                if (_chickens > 0)
                {
                    Random randomEggCount = new Random();
                    int eggsLaid = randomEggCount.Next(_chickens) + 1;
                    _eggs += eggsLaid;
                    _nextEggLaid = DateTime.Now.AddMinutes(15);
                    if (eggsLaid > 0)
                    {
                        foreach (Player player in entityManager.GetComponents<Player>()) // Line 74
                        {
                            player.PlayOpenCdnStream("sfx", "chickenEggPop.mp3", _objectLinkedTo.Position, 3.0f);
                        }
                    }
                }
            }
            UpdateTextLabel();
        }

It's not unthinkable an exception could occur in OnPlayerDisconnect, but this is also the only OnPlayerDisconnect in my code that I have at this moment:

        [Event]
        public void OnPlayerDisconnect(Player player, DisconnectReason disconnectReason, IDiscordService discordService)
        {
            #if (!DEBUG)
                if (player.IsPlayerPlayingAsCharacter())
                {
                    discordService.SendGeneralChatMessage($"## {player.Name.Replace("_", " ")} is no longer playing on the server.");
                }
            #endif
        }

It's just strange that I don't see any mention of this in the exceptions. At the time of execution, Player should still exist considering the message with Player name does show up in Discord.

@ikkentim
Copy link
Owner

ikkentim commented Jan 27, 2025

The previously mentioned exception might occur due to not being executed on the main thread. maybe using a system timer could help resolve that.

[Timer(60000)]
public void Every60Sec(IServerService serverService)
{
    Console.WriteLine($"Every 60 seconds timer! tick rate: {serverService.TickRate}");
}

That OnPlayerDisconnect handler indeed doesn't seem very likely to cause these issues (though they could if the discord api is malfunctioning?). To make it totally error proof, you could add a try-catch around it.
I can't really see an issue in the code I've seen so far.

Maybe you can add some debug logging to an OnPlayerDisconnect handler: enumerate over entityManager.GetComponents<Player>() and log their names and the total count

@KoertLichtendonk
Copy link
Author

KoertLichtendonk commented Jan 27, 2025

Didn't know about the Timer attribute being part of SampSharp, thanks for teaching me something new! 😊

As for the try-catch, wouldn’t it make more sense to have it implemented in SampSharp itself at the point where all the individual system events such as OnPlayerDisconnected are triggered? Or is not doing so an intentional design choice?

Either way, I'll add a try-catch around the code in the OnPlayerDisconnect and build in that extra logging to enumerate over entityManager.GetComponents() and check how many entities exist if this issue happens again. It’s the first time I’ve encountered something like this, and restarting the server fixed it immediately. Hopefully, with these changes, I can pinpoint the cause more easily if it recurs. Thanks for the input!

@ikkentim
Copy link
Owner

As for the try-catch, wouldn’t it make more sense to have it implemented in SampSharp itself at the point where all the individual system events such as OnPlayerDisconnected are triggered? Or is not doing so an intentional design choice?

yes it would indeed, but this will be quicker for now

@ikkentim
Copy link
Owner

ikkentim commented Mar 1, 2025

@KoertLichtendonk I've added a try/finally in the 0.11.0 update. ae3973f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants