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

Revscriptsys v2 #4629

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Revscriptsys v2 #4629

wants to merge 11 commits into from

Conversation

EvilHero90
Copy link
Contributor

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:

  • converted all raw pointers to shared_ptr from the interfaces (Action, GlobalEvents, etc.)
  • remove xml registering for interfaces (Action, GlobalEvents, etc.)
  • vocations can be created and maintained by lua.
  • groups can be created and maintained by lua.
  • lib folders are merged and auto load like the scripts folder.
  • better and safer reload system (detailed explanation scroll down)
  • error messages on false setup instead of c++ exceptions.
  • easier setup of scripts, since we don't have to manually call register() anymore.

Features version 2 wont include:

  • npc's in lua (want to create a new npc system first)

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!

- unification of both `lib` folders, making them auto load the files instead of manual linking
- moved global.lua into `lib` folder
- removed the `lib` folder inside `scripts` since it has no use anymore

revscriptsys relevant:
- re worked the `Game::reload()` function since it's kinda redundant for the new version of revscriptsys as we can re load specific events by their function / file name instead of reloading an entire interface
- basic ground work to work on the interfaces now
- re named `RELOAD_TYPE_GLOBAL` to `RELOAD_TYPE_LIBRARY` as the name is far more descriptive
- removed xml related stuff
- raw action pointers are now converted into shared pointers
- added basic debugging that server owner gets notified if he uses an outdated script version of revscriptsys
- added `Game.getAction(id, eventType) to be able to hook back into an already created function and be able to edit/copy it
- print in console which revscriptsys version we are using
- removing all not necessary xml handling code
- introducing shared_ptr for Spell/InstantSpell/RuneSpell
- we keep monster xml spell handling as we still use monster xml files
-
- removed all xml related stuff
- converted everything to shared_ptr
- notification if we use an outdated revscriptsys version
@EvilHero90 EvilHero90 added enhancement Increase or improvement in quality, value, or extent needs-triage Needs testing with screenshot/video confirmation feature New feature or functionality labels Feb 15, 2024
@EvilHero90 EvilHero90 added this to the 1.6 milestone Feb 15, 2024
@MillhioreBT
Copy link
Contributor

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Increase or improvement in quality, value, or extent feature New feature or functionality needs-triage Needs testing with screenshot/video confirmation
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

2 participants