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

[RFC] Scripting API #59

Open
Deewarz opened this issue Oct 19, 2021 · 8 comments
Open

[RFC] Scripting API #59

Deewarz opened this issue Oct 19, 2021 · 8 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Deewarz
Copy link
Contributor

Deewarz commented Oct 19, 2021

Summary

The objective of this RFC (Request for Comments) is to jointly define the vision for Scripting API.

I have already thought about it and I would like to share you an example.

It's possible that this implementation is complicated to implement or requires a JS wrapper on top of the NAPI layer.
That's why I'm starting this thread to see together what can be done.

For now, I focus on the server side.

Goals

  • OOP first API
  • Obvioulsy, inheritance (eg. Player class extends of Human class extends of Entity)
  • Typed API to benefit from auto-completion and error detection
  • Never instantiate but always use a factory (do World.createVehicle(...) instead of new Vehicle(...))
  • All "scripting modules" can listen for event (and some of them can emit) (also to net)
  • You can attach data to entities (and you can synchronize them with the client)
  • All methods are, or start with, a verb. (getXX, setXX, spawn, ...) (don't player.position but player.getPosition)
  • We use properties for immutable values (eg. player.id)

Example

import {
    Resources, // Resources scripting module
    Players, // Players scripting module
    Commands, // Commands scripting module
    Vehicles, // Vehicles scripting module
    Assets, // Assets definitions (data)
    Enums, // Enums definitions
    Vector3, // Vector3 factory
    World, // World scripting module
    Chat, // Chat scripting module
} from '@mafiahub/mafiamp/server' // could be shorter?

/**
 * You can also import ES module directly
 **/
// import Resources from '@mafiahub/mafiamp/server/resources'
// import WeaponsAssets from '@mafiahub/mafiamp/server/assets/weapons'



/*******************************************
 *
 * Resources scripting module example
 *
 *******************************************/
Resources.on('loaded', ({ resource }) => { // Always an object as argument to keep it scalable and for the end user, choose what he wants and in the order he wants
    console.log(`${resource.name} is loaded`)
})

Resources.on('unloading', ({ resource }) => {
    console.log(`${resource.name} is unloading.`)
})


/**
 * This function is an helper reused in
 * player connected and player dead events
 **/
function initPlayer (player) {
    player.setPosition(Vector3(0, 0, 0))
    player.setDirection(Vector3(0, 0, 0))
    player.setModel(/* TBD */)

    const { modelName, maxAmmo } = Assets.Weapons.getById(1)
    player.giveWeapon(modelName, maxAmmo) // can be discussed due to the way MDE works

    player.spawn()
}


/*******************************************
 *
 * Players scripting module example
 *
 *******************************************/
Players.on('connected', ({ player }) => { // We receive the player instance directly
    const message = `${player.name} (${player.id}) is connected.`
    console.log(message)
    player.sendChatToAll(message, 0, 0, 0, 1)

    // Here you can get data from database

    player.setData('isAdmin', true) // you can attach data to entity

    initPlayer(player)
})

Players.on('disconnecting', ({ player }) => {
    console.log(`${player.name} (${player.id}) is disconnecting.`)

    // Here you can save data in database

    player.sendChatToAll(`${player.name} (${player.id}) is disconnected.`, 0, 0, 0, 1) // for other players we tell them the player is already disconnected.
})

Players.on('spawned', ({ player }) => {
    console.log(`${player.name} (${player.id}) is spawned.`)

    player.emit('myCustomEvent', { foo: 'bar' }) // player emit a custom event with custom properties (see the handler below)
    player.emitNet('myEventFromServerToClient', { foo: 'bar' }) // this event will be emitted to the client

Players.on('myCustomEvent', ({ player, foo }) => { // merge base properties (player) with the emitted (foo)
    console.log(`${player.name} (${player.id}) send a custom event with ${foo} `)
})

Players.once('myCustomEvent', ({ player, foo }) => { // this uses "once", it means that the callback will be executed once
    console.log('This will be executed once.', player.name, foo)
})

Players.on('died', ({ player, killer }) => { // killer (if any) is an entity instance
    const killerType = killer?.type // returns the type of the entity or null

    switch (killerType) { // not exhaustive
        case Enums.ENTITY_TYPES.PLAYER:
            console.log(`${player.name} (${player.id}) was killed by ${killer.name}.`)

        case Enums.ENTITY_TYPES.CAR:
            const killerPlayer = killer.getInSeat(0)
            console.log(`${player.name} (${player.id}) was killed by a car driven by ${killerPlayer.name}.`)

        default:
            console.log(`${player.name} (${player.id}) is dead.`)
    }

    player.resetInventory()
    initPlayer(player)
})


/*******************************************
 *
 * Commands scripting module example
 *
 *******************************************/
Commands.on('mycommand', async ({
    player, // always pass the player instance
    command // always pass the command instance
}) => {
    console.log(command.name) // print "mycommand"
    console.log(command.args) // args is always defined as an Array[] even if there is no arguments, so you can use destructuring safely. (eg. `const [id, test, bar, foo] = command.args`)
    // command.listener() (it's this callback)

}, "I'm the help message.") // as 3rd parameters, you can provide an help message


Commands.on('car', async ({ player, command }) => {
    const id = parseInt(command.args[0], 10)

    if (!Number.isInteger(id)) {
        return player.sendChat(`You must provide a vehicle id.`, 255, 0, 0, 1)
    }

    const vehicleData = Assets.Vehicles.getById(id)

    if (!vehicleData) {
        return player.sendChat(`Unable to find vehicle with id: ${id}.`, 255, 0, 0, 1)
    }

    const { modelName } = vehicleData
    const vehicle = await World.createVehicle(modelName, /* position */ Vector3(0, 0, 0), /* direction */ Vector3(0, 0, 0), /* isVisible */ false) // This should be a promise?

    vehicle.setPrimaryColor(255, 255, 255, 1) // RGBA
    vehicle.setVisible(true)

    player.putInVehicle(vehicle)
}, "I'm the help message.") // as 3rd parameters, you can provide an help message

Commands.on('weapon', ({ player, command }) => {
    const id = parseInt(command.args[0], 10)

    if (!Number.isInteger(id)) {
        return player.sendChat(`You must provide a weapon id.`, 255, 0, 0, 1)
    }

    const weaponData = Assets.Weapons.getById(id)

    if (!weaponData) {
        return player.sendChat(`Unable to find weapon with id: ${id}.`, 255, 0, 0, 1)
    }

    const { modelName, maxAmmo } = weaponData
    player.giveWeapon(modelName, maxAmmo)
})

Commands.on('alert', ({ player, command }) => {
    if (!player.getData('isAdmin')) {
        return player.sendChat(`You are not allowed to send a server alert.`, 255, 0, 0, 1)
    }

    const [message] = command.args

    if (!message) {
        return player.sendChat(`You must provide a message.`, 255, 0, 0, 1)
    }

    Chat.sendToAll(`[SERVER] ${message}`, 0, 0, 0, 1) // message as the server
})

Commands.on('delallveh', ({ player }) => {
    if (!player.getData('isAdmin')) {
        return player.sendChat(`You are not allowed to use this command.`, 255, 0, 0, 1)
    }

    const vehicleList = Vehicles.getList()
    vehicleList.forEach((veh) => veh.delete())

    Chat.sendToAll(`[SERVER] All vehicles are deleted.`, 0, 0, 0, 1)
})

Commands.on('players', ({ player }) => {
    const players = Players.getList()

    const message = players.map(player => {
        return `- ${player.name}(${player.id})\n`
    })

    player.sendChat(message, 0, 0, 0, 1)
})

Commands.on('goto', ({ player, command }) => {
    const targetId = parseInt(command.args[0], 10)

    if (!Number.isInteger(id)) {
        return player.sendChat(`You must provide a player id.`, 255, 0, 0, 1)
    }

    const targetPlayer = Players.getById(targetId) // returns the instance of this player

    if (!targetPlayer) {
        return player.sendChat(`Unable to find player with id: ${targetId}.`, 255, 0, 0, 1)
    }

    player.setPosition(targetPlayer.getPosition()) // targetPlayer.getOffsetPosition()
})

Commands.on('repair', ({ player }) => {
    const vehicle = player.getCurrentVehicle()

    if (!vehicle) {
        return player.sendChat(`You must be in a vehicle.`, 255, 0, 0, 1)
    }

    vehicle.repair()
})

Commands.on('color', ({ player }) => {
    const vehicle = player.getCurrentVehicle()

    if (!vehicle) {
        return player.sendChat(`You must be in a vehicle.`, 255, 0, 0, 1)
    }

    vehicle.setColor(0, 0, 0, 1, 0, 0, 0, 1) // primary (setPrimaryColor) + secondary (setSecondaryColor)
})

Commands.on('help', ({ player }) => {
    const commandList = Commands.getList()

    if (commandList.length === 0) {
        return player.sendChat(`There is no command.`, 255, 0, 0, 1)
    }

    const message = []

    commandList.forEach((cmd) => {
        message.push(`- /${cmd.name} ${cmd.helpMessage && ('-- ' + cmd.helpMessage)}`)
    })

    player.sendChat(message.join('\n'), 255, 0, 0, 1)
})


/*******************************************
 *
 * Chat scripting module example
 *
 *******************************************/
Chat.on('message', ({ player, message }) => { // catch all chat messages
    if (player) {
        return console.log(`New message from player ${player.name} (${player.id}):`, message)
    }

    console.log(`New message from server:`, message)
})
@martonp96
Copy link
Collaborator

Hello @Deewarz ,

This is a nicely written issue. Can you please elabore on the object as argument part? I'm not sure if i understand the reasoning for this.

@Deewarz
Copy link
Contributor Author

Deewarz commented Oct 19, 2021

Hello @Deewarz ,

This is a nicely written issue. Can you please elabore on the object as argument part? I'm not sure if i understand the reasoning for this.

Hello @martonp96, thank you for your comment!

I mean you only get a single argument for your listener (which we could call the context (ctx) for example)
The context argument is always an object that can receive different keys, which allows destructuring.

So, it's as if I had written:

Resources.on('loaded', (ctx) => {
    const { resource } = ctx
    console.log(`${resource.name} is loaded`)
})

A first example to understand the interest with the Commands module.
In the context, you get the player instance and the command instance.

If you don't need the player, you don't have to write it in your code:

Commands.on('test', ({ command }) => {
    console.log(`${command.name} is executed.`)
})

2nd example with a custom event where I would only need some of the arguments:

// client side
player.emitNet('myEvent', { foo: true, bar: false, plop: 1})


// server side
Players.on('myEvent', ({ foo }) => { // ignore player, bar, plop
    console.log(foo)
})

Players.on('myEvent', ({ plop }) => { // ignore player, foo, bar
    console.log(plop)
})

// Otherwise i should define arguments that I don't need
Players.on('myEvent', (player, foo, bar, plop) => { // i don't need player, foo, bar
    console.log(plop)
})

Finally, there is also an interest for the Scripting API maintainers because they can add things to the context of a module listener without causing a breaking change in the signature of the listener.

Have I been clear enough? Otherwise do not hesitate to ask :)

@LeonMrBonnie
Copy link
Collaborator

Finally, there is also an interest for the Scripting API maintainers because they can add things to the context of a module listener without causing a breaking change in the signature of the listener.

This can also be done by adding new arguments to the end of the argument list.
I think this makes the API look really different from the usual JS library experience where you get multiple arguments, though this is of course something where everyone has a different taste.
I just think that this principle of only passing one argument which is an object, is not really nice style.

All methods is, or starts with, a verb. (getXX, setXX, spawn, ...) (don't player.position but player.getPosition)

Whats the reason for doing this? This is of course again just taste, but I think getters and setters are a nice feature of JS that should be utilized here. The getX and setX approach is just what you would have used in older languages (which don't have setters / getters like C++ etc.) so maybe this is just something out of habit.

@inlife
Copy link
Member

inlife commented Oct 20, 2021

@Deewarz is it inspired by some existing solution?

@inlife
Copy link
Member

inlife commented Oct 20, 2021

This can also be done by adding new arguments to the end of the argument list.
I think this makes the API look really different from the usual JS library experience where you get multiple arguments, though this is of course something where everyone has a different taste.
I just think that this principle of only passing one argument which is an object, is not really nice style.

@LeonMrBonnie I think it was definitely a case before. But seems nowadays it is indeed more common to see the object unpacking mechanism, specifically for event arguments, just like @Deewarz pointed out. Also it makes sense from the other side when that same object is being the argument for the event transmitter.

@Deewarz
Copy link
Contributor Author

Deewarz commented Oct 20, 2021

@LeonMrBonnie, @inlife Thanks for your comments!


To @LeonMrBonnie + @inlife:

This can also be done by adding new arguments to the end of the argument list.
I think this makes the API look really different from the usual JS library experience where you get multiple arguments, though this is of course something where everyone has a different taste.
I just think that this principle of only passing one argument which is an object, is not really nice style.

@LeonMrBonnie I think it was definitely a case before. But seems nowadays it is indeed more common to see the object unpacking mechanism, specifically for event arguments, just like @Deewarz pointed out. Also it makes sense from the other side when that same object is being the argument for the event transmitter.


To @LeonMrBonnie:

All methods are, or start with, a verb. (getXX, setXX, spawn, ...) (don't player.position but player.getPosition)

Whats the reason for doing this? This is of course again just taste, but I think getters and setters are a nice feature of JS that should be utilized here. The getX and setX approach is just what you would have used in older languages (which don't have setters / getters like C++ etc.) so maybe this is just something out of habit.

Originally, I had considered using the Getter & Setter, but I changed my mind because it is more explicit to do it this way for two main reasons:

  • We can differentiate immutable properties more easily
  • Client side, you mostly have only getter, by prefixing the methods it's more explicit to know the use of the method

For example, I want to manipulate the player position:

  • Server side, I have player.setPosition() and player.getPosition() methods
  • Client side, I have only player.getPosition() method

If I use the native get / set I only have player.position (I don't explicitly know if I can use the setter or not)

Finally, the choice to always use a verb is mostly to keep consistency in API (eg. player.sendChat() instead of player.chat())


To @inlife:

@Deewarz is it inspired by some existing solution?

Not really, not an existing API but simply inspired by my professional experience and my thoughts on other scripting APIs from other games.

@zpl-zak
Copy link
Member

zpl-zak commented Oct 21, 2021

Hi, thank you for the RFC contrib!

Never instantiate but always use a factory (do World.createVehicle(...) instead of new Vehicle(...))

At first, I wasn't sure if this rule is necessary, but when I look at it from the user's POV, this rule ensures that we make it explicit the object created is not owned by the script itself but rather by the MP layer the request is queried to that provides us a handle to it. This can clear out confusion where one might consider storing the object instance in the event of a possible GC, which in reality does not affect the data itself. By establishing a rule only to allow object creation via factories, we help users know the MP side owns the data and the script only retrieves a handle to it.

All "scripting modules" can listen for events (and some of them can emit) (also to the net)

This slightly contradicts the idea behind OOP first API as the events should actually be overriding base event listeners to extend functionality:

MyModPlayer.prototype.died = ctx -> {}

However, given the nature of the scripting API, the NAPI part is not aware of a script-level inherited class that would provide such extension (e.g. NAPI doesn't know MyModPlayer class is a thing unless we explicitly state that on script-level), this is a very fair compromise and the use of message-based event processing via a static method very much makes sense. This solution is the equivalent of passing the object instance into a static class member method and should work well in that case.

tl;dr, this is a good approach.

You can attach data to entities (and you can synchronize them with the client)

This is a good idea. NAPI objects (such as: Player, Vehicle instances) only serve as reference objects holding the bare minimum of data to identify their native persistent counterpart. The ability to store custom data on the mp-level side (via setData) also allows us to transmit and share data across multiple scripting instances or even over the network.

All methods are, or start with, a verb. (getXX, setXX, spawn, ...) (don't player.position but player.getPosition)

This is a valid proposition. Even if properties would make more sense from a syntax standpoint, semantically, it makes more sense if data is accessed via regular getters/setters.

Apart from what you've already mentioned, the reasoning behind this also goes into an understanding of how to work with the data NAPI provides us. The MP doesn't serve us a full-blown representation of a native object. It merely provides us with an object that refers/identifies it.

Properties assume the data you work with is persistent, but that isn't the case since you don't work with the raw native object. Having explicit getters/setters help greatly as it establishes a strong division between what we should consider as immutable temporal data served only to identify native data or what we present as native data on its own.

Consistency also plays a role in establishing a strong API which ensures the API user instinctively knows how to access and manipulate the data provided.

We use properties for immutable values (eg. player.id)

This supports the previous statement, as it marks a clear division between native data representation and the mere reference to it. I would, however, consider reducing this scope purely into identification data necessary to communicate with the native counterpart.

@zpl-zak
Copy link
Member

zpl-zak commented Oct 26, 2021

Is there anything else we could discuss?

NOTE: I'm moving this issue over to the Framework as MafiaMP depends on it directly.

@zpl-zak zpl-zak transferred this issue from MafiaHub/MafiaMP Oct 26, 2021
@inlife inlife added documentation Improvements or additions to documentation enhancement New feature or request labels Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: No status
Status: No status
Development

No branches or pull requests

5 participants