MafiaHub/MafiaMP

TypeError: player.getNickname is not a function

Closed this issue · 4 comments

Issue

TypeError: player.getNickname is not a function upon connection gets randomly thrown when we try to welcome a new player in a gamemode script.

Resources

WindowsTerminal_SQmXlbXQBl

The issue happens in https://github.com/MafiaHub/Framework/blob/develop/code/framework/src/scripting/engines/node/engine.h#L88 where any objects created using isolate right before the event call somehow get occasionally passed into a function call arg stack.

I think it's related to the way we use the V8::Locker here.

For the player, it's also locked in WrapHuman via V8_RESOURCE_LOCK.

We must "isolate" and "handle" scope in each function but lock only one time per callback.

I think the whole callback lifecycle needs a refacto to make it easier to manipulate, but I'm not good enough at C++ to do it.


The dumb solution would be to remove the locker in InvokeEvent and in the WrapHuman function and do the operation in each event callbacks.

Eg.

        static v8::Local<v8::Object> WrapHuman(Framework::Scripting::Engines::Node::Engine *engine, flecs::entity e) {
            return v8pp::class_<Scripting::Human>::create_object(engine->GetIsolate(), e.id());
        }
        
        static void EventPlayerConnected(flecs::entity e) {
            auto engine = reinterpret_cast<Framework::Scripting::Engines::Node::Engine*>(Framework::CoreModules::GetScriptingModule()->GetEngine());

            V8_RESOURCE_LOCK(engine);

            auto playerObj = WrapHuman(engine, e);

            engine->InvokeEvent("playerConnected", playerObj);
        }
        template <typename... Args>
        void InvokeEvent(const std::string name, Args &&...args) {
            v8::Isolate::Scope isolateScope(GetIsolate());
            v8::HandleScope handleScope(GetIsolate());
            v8::Context::Scope contextScope(_context.Get(_isolate));

            if (_gamemodeEventHandlers[name].empty()) {
                return;
            }
            
            ...

It's worth noting not only the function of getting nickname, but also the others, like getting health or setting position.
In general, it looks like the interaction with the player is completely broken...

Also, it might be great to be able to use InvokeEvent like this: (as discussed on this issue: MafiaHub/Framework#59)

InvokeEvent("playerConnected", { {"player", playerObj}, {"foo", true}, {"bar", 1} });

In order to use it on scripting side like :

sdk.on("playerConnected", ({ player, foo, bar }) => {}

Warning: We have to support no args and maybe non map too.


Casting in InvokeEvent could be done with v8pp::to_v8


Here is a badly written and non working example generated via ChatGPT: (and no support for no args or non map)

template <typename CtxType>
        void InvokeEventV3(
            const std::string name,
            const std::unordered_map<std::string, CtxType>& eventCallbackCtx = {}
            // const std::unordered_map<std::string, v8::Local<v8::Object>>& eventCallbackCtx = {}
        ) {
            if (_gamemodeEventHandlers[name].empty()) {
                return;
            }

            v8::Isolate::Scope isolateScope(GetIsolate());
            v8::HandleScope handleScope(GetIsolate());
            v8::Context::Scope contextScope(_context.Get(_isolate));

            v8::Local<v8::Value> v8_args[1];

            bool eventCallbackCtxIsEmpty = eventCallbackCtx.empty();

            if (!eventCallbackCtxIsEmpty) {
                auto context = _context.Get(_isolate); // TODO: can we declare context once before  v8::Context::Scope contextScope(_context.Get(_isolate));

                v8::Local<v8::Object> eventCallbackCtxV8Obj = v8::Object::New(_isolate);

                for (const auto& [key, value] : eventCallbackCtx) {
                    eventCallbackCtxV8Obj->Set(_isolate->GetCurrentContext(), v8pp::to_v8(_isolate, key), to_v8(_isolate, value));
                }

                v8_args[0] = v8pp::to_v8(_isolate, eventCallbackCtxV8Obj);
            }

            for (auto it = _gamemodeEventHandlers[name].begin(); it != _gamemodeEventHandlers[name].end(); ++it) {
                v8::TryCatch tryCatch(_isolate);

                if (eventCallbackCtxIsEmpty) {
                    it->Get(_isolate)->Call(_context.Get(_isolate), v8::Undefined(_isolate), 0, nullptr);
                } else {
                    it->Get(_isolate)->Call(_context.Get(_isolate), v8::Undefined(_isolate), 1, v8_args); // TODO: check what happens if I pass eventCallbackCtxV8Obj directly
                }

                if (tryCatch.HasCaught()) {
                    auto context = _context.Get(_isolate);
                    v8::Local<v8::Message> message = tryCatch.Message();
                    v8::Local<v8::Value> exception = tryCatch.Exception();
                    v8::MaybeLocal<v8::String> maybeSourceLine = message->GetSourceLine(context);
                    v8::Maybe<int32_t> line                    = message->GetLineNumber(context);
                    v8::ScriptOrigin origin                    = message->GetScriptOrigin();
                    Framework::Logging::GetInstance()->Get(FRAMEWORK_INNER_SCRIPTING)->debug("[Helpers] Exception at {}: {}:{}", name, *v8::String::Utf8Value(_isolate, origin.ResourceName()), line.ToChecked());

                    auto stackTrace = tryCatch.StackTrace(context);
                    Framework::Logging::GetInstance()->Get(FRAMEWORK_INNER_SCRIPTING)->debug("[Helpers] Stack trace: {}", *v8::String::Utf8Value(_isolate, stackTrace.ToLocalChecked()));
                }
            }
        }