otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.57k stars 1.05k forks source link

Revscriptsys v2 #4629

Open EvilHero90 opened 6 months ago

EvilHero90 commented 6 months ago

This implementation of revscriptsystem of version 2 is a more robust and error forgiving update to the old system. Now to list a few key features version 2 comes with:

Features version 2 wont include:

Let's get into some details and examples on how version 2 works.

Script Examples

Action:

revscriptsys v1:

local test = Action()

function test.onUse(player, item, fromPosition, target, toPosition, isHotkey)
    return true
end

test:id(1,2,3,4,5,6)
test:aid(1,2,3,4,5,6)
test:uid(1,2,3,4,5,6)
test:allowFarUse(true)
test:blockWalls(true)
test:checkFloor(true)
test:register()

revscriptsys v2:

local config = {
    -- the key has to match the functions which are for the Action class
    -- since we directly parse the keys as functions and the values as the parameters
    id = {1,2,3,4,5,6},
    -- single id
    id = 1,
    aid = {1,2,3,4,5,6},
    uid = {1,2,3,4,5,6},
    allowFarUse = true,
    blockWalls = true,
    checkFloor = true
}

local test = Action(config)

-- We have to name the function now unique in order to be able to reload them without problems (still starting with onUse, this way it knows that it's an onUse script)
-- this is a huge enhancement to current reload system, as this makes it possible to reload single events instead of an entire interface
function test.onUseNewTest(player, item, fromPosition, target, toPosition, isHotkey)
    return true
end

-- we don't need to register in this case, since it does all the registering once we hook the event function

Reload System

Since we are using uniquely named function names (ex: onUseNewTest1) we can finaly revamp our reloading system. Currently we can only reload an entire interface (ex: /reload actions) or reload the entire scripts folder which sometimes gives bad outcome, since it can reload tables or other stuff you don't want to be cleared at this point. Now you are able to reload easily a single interface function or an entire file by doing either /reload onUseTest1 or /reload rope.lua there is no need to give it a path, it'll find it without problems without providing that.

This PR is still a draft and there are more examples comming soon. Feel free to ask questions or suggest improvements!

MillhioreBT commented 6 months ago

I'm very glad you're back, and it's interesting what you've been doing with RevScript...

However, I don't quite understand what the benefits of this new way would be? Ok if it is true we can reload a single specific function...

Eliminating register is not a bad idea but it would no longer be consistent with EventCallback.

The current way of doing it is quite convenient, and doesn't have much magic behind it, it's just raw functions and a __newindex metamethod to choose the event based on the name.

What does catch my attention is this: image

Removing XML support is not necessary, but I think no one would mind not having it, right?

EvilHero90 commented 6 months ago

I'm very glad you're back, and it's interesting what you've been doing with RevScript...

However, I don't quite understand what the benefits of this new way would be? Ok if it is true we can reload a single specific function...

It's more of a stability factor than a mere feature implementation. We have another easier option to setup a script file this way, with a rather detailed explanation on what is missing or badly setup if you forgot something. The single reload function/file is just a benefit.

Eliminating register is not a bad idea but it would no longer be consistent with EventCallback.

It's not entirely eliminated, we just don't have to manualy use it anymore. We can still write scripts like in v1 without any problems (but with uniquely named functions), both ways are enabled.

The current way of doing it is quite convenient, and doesn't have much magic behind it, it's just raw functions and a __newindex metamethod to choose the event based on the name.

What does catch my attention is this: image

I'm steadily working on that but we all know good things take time and writing a npc system from scratch needs a lot of testing first. Removing XML support is not necessary, but I think no one would mind not having it, right?

In this case it's necessary because it doesn't work with the way we are registering the uniquely named functions anymore. There are no plans to remove XML entirely, just for the Interfaces I'm revamping right now. Vocations and such will be accessable through XML and lua in this case then.