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] RevNpcSys version 1.0 (lua npcs / NpcType / new npc system) #4671

Merged
merged 52 commits into from
May 30, 2024

Conversation

EvilHero90
Copy link
Contributor

Pull Request Prelude

Changes Proposed

Finaly introducing lua npcs with an oop based new npc system.
Both systems lua and xml are now based on NpcType.
This pr doesn't change anything on the already existing xml npcs and jiddos npc system, that's still valid and functional.
Fixed a memory leak regarding xml npcs.
I've included 2 example npcs in data/npc/lua/#test.lua

Issues addressed:
none

EvilHero90 and others added 17 commits April 30, 2024 15:57
- adding lua npcs handling
- tree handling talkstates
- a few bugfixes and corrections
- added callbacks to keywords
- adding storagevalue requirements for easy checks with failure respond function
- added documentation and annotations
- split all the functions in different files for better overview and organization
- if you add a callback to a keyword it can return false and also return a string for the failure respond message
- changed order of calling functions in events
1) checks callback
2) checks requirements
3) opens shop
now we are not opening shop even tho requirements or callback was not correct
- added an example usage of callback in handler.lua
- corrections in documentation
- constants.lua
adding more messages to MESSAGE_LIST
- events.lua
fixing a problem where the player could say keywords without greeting the npc first
added requirements
added a `help` default keyword, which will tell the player to which keywords the npc responds
- general better documentation
- added modules.lua
- added requirements.lua
- added a test example npc data/npc/lua/#test.lua
- removed `requireStorage` it's redundant as we have the same in requirements
- renamed
`setGreetRespond` -> `setGreetResponse`
`setFarewellRespnd` -> `setFarewellResponse`
- added
`setGreetKeywords`
`setFarewellKeywords`
- fixed annotations
- moved everything to NpcType
- we don't create a NpcScriptInterface per npc created rather just load it by each NpcType
-
- we are sharing one NpcScriptInterface instead of creating one for each npc
forgot to include

Co-Authored-By: Sarah Wesker <sarahelizabetwesker@gmail.com>
the usual
src/luascript.cpp Fixed Show fixed Hide fixed
src/npc.h Dismissed Show dismissed Hide dismissed
src/npc.cpp Dismissed Show dismissed Hide dismissed
@EvilHero90 EvilHero90 added enhancement Increase or improvement in quality, value, or extent feature New feature or functionality labels May 7, 2024
@EvilHero90 EvilHero90 added this to the 1.6 milestone May 7, 2024
- `keyword` and `respond` support now multiple inserts via table
- if there is multiple keywords for the same outcome we just point to one metatable instead of creating several (memory efficient)
- made all the get functions of `NpcType` into const
- added `NpcType.onMove`,`NpcType.onPlayerCloseChannel` and `NpcType.onPlayerEndTrade` with their callback functions
- correction for lua npcs at ~NpcEventsHandler() which caused them to have no scripting interface when an npc was removed and respawned
- you can now use item names aswell for shop items
- we are sending errors in console if the item does not exist or if it doesn't have the property to be able to pick it up
- adding `NpcsHandler:farewell()` function to release focus on yourself on certain keywords.
- fixed correct order of trying to get a failureResponse when working with callbacks
- shows an error message if there is no set failureResponse
- changing npcEventHandler shared_ptr to unique_ptr
Copy link
Member

@ranisalt ranisalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool to have Lua NPCs, it's one step further away from XML. I don't have enough knowledge to do a proper review here though.

data/lib/compat/compat.lua Show resolved Hide resolved
@EvilHero90
Copy link
Contributor Author

Really cool to have Lua NPCs, it's one step further away from XML. I don't have enough knowledge to do a proper review here though.

Feel free to just review the .cpp/h changes, those are mostly the critical changes, we still have the old npc system if something shouldn't correctly work at some point I can just push fixes and increase the version

src/luascript.h Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
src/npc.h Outdated Show resolved Hide resolved
- removed info struct and moved everything to public
@EvilHero90 EvilHero90 requested a review from ranisalt May 23, 2024 11:20
- delay is now global and was removed out of the voices table, default delay is 30s and the delay can be set at handler:talk(voices, delay)
- we check now if the voice it wants to say is the same as the last voice and then roll again until it finds another one if there are several to choose from
This now saves the player who was sending the money regardless if the receiver is online/offline and does a db query for the receiver to keep the bank balance in sync
ranisalt
ranisalt previously approved these changes May 25, 2024
Copy link
Member

@ranisalt ranisalt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, let's see what @MillhioreBT and @ramon-bernardo have to say

Copy link
Contributor

@ramon-bernardo ramon-bernardo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like having to perform an update/save every time a player transfers gold. We should assume that the server will NEVER crash. However, if it's okay with this, it LGTM.

renaming folder from evilnpcsystem to revnpcsys
ranisalt
ranisalt previously approved these changes May 26, 2024
EvilHero90 and others added 7 commits May 27, 2024 17:30
Co-Authored-By: Sarah Wesker <sarahelizabetwesker@gmail.com>
The npc was not correctly focusing players when they talked to him, this bug is now fixed.

Co-Authored-By: Sarah Wesker <sarahelizabetwesker@gmail.com>
now you can check in requirements for certain vocations in order to pass
- added some linter corrections and additions
- changed player:getItemCount(id, subType) -> player:getItemCount(id, subType, ignoreEquipped) that way we can exclude equipped items or not
- added NpcsHandler:addItems(table--[[,container, inbox]])
we can add a table of items with that, if the player doesn't have enough space or capacity then the items will be send to inbox
- added Player.sendInboxItems(self, items, containerId)
- re worked onStorageValue callback
we can now set a "range" with ex:
onStorageValue(1, ">", 10, "<")
- re worked requirements (level & storagevalue) checks, they accept now ranged checks aswell
level(1, ">=", 9, "<=")
storage(10, ">", 50, "<")
- changed NpcsHandler:teleport(pos) -> NpcsHandler:teleport(pos, magicEffectFromPos, magicEffectToPos)
default is CONST_ME_TELEPORT
- added NpcType:health(...) & NpcType:maxHealth(...) functions
-added NpcRequirements:items(table) & NpcRequirements:removeItems(table)
- added some stuff in constants.lua
@EvilHero90 EvilHero90 changed the title [Revscriptsys] lua Npcs / NpcType & new Npc system [Revscriptsys] RevNpcSys version 1.0 May 30, 2024
@EvilHero90 EvilHero90 changed the title [Revscriptsys] RevNpcSys version 1.0 [Revscriptsys] RevNpcSys version 1.0 (lua npcs / NpcType / new npc system) May 30, 2024
Copy link
Contributor

@MillhioreBT MillhioreBT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@EvilHero90 EvilHero90 merged commit 4a89502 into otland:master May 30, 2024
19 checks passed
@ranisalt
Copy link
Member

Let's go 🚀

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants