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

[Core/Event Scripts] Implement a new way to handle events #18873

Closed
Rushor opened this issue Jan 17, 2017 · 17 comments
Closed

[Core/Event Scripts] Implement a new way to handle events #18873

Rushor opened this issue Jan 17, 2017 · 17 comments

Comments

@Rushor
Copy link
Contributor

Rushor commented Jan 17, 2017

We all know SAI is very bad to script "out-of-combat" events, especially if serveral different gameobject guids or creature guids handle different events based on their guid.

A better way would be to add one central table that handles all events. The event itself must be linked to a specific type (in gameobject use, on spell hit, on creature id out of combat, on creature guid out of combat and so on...).

I'm thinking of a table which works similar to waypoint/spell/event _scripts, but is combined together.

Any ideas on how it should be improved are welcome. I will update the template then.

Example what it should look like:

https://gist.github.com/Rushor/a17b945bcad413d822b6fb19efb0fd68

SET @TYPE_SPELL_HIT         := 1;
SET @TYPE_GO_USE            := 2;
SET @TYPE_CREATURE_ENTRY    := 3;
SET @TYPE_CREATURE_GUID     := 4;
SET @TYPE_QUEST_START       := 5;
SET @TYPE_QUEST_END         := 6;
SET @TYPE_ON_CREATURE_DEATH := 7;

DELETE FROM `event_script_linking_template`;
INSERT INTO `event_script_linking_template` (`type`, `source`, `scriptid`, `start_time_min`, `start_time_max`, `repeat_time_min`, `repeat_time_max`) VALUES 
(@TYPE_SPELL,            46802, 4680200, 3000, 3000,     0,     0), -- starts event after 3 seconds after spellhit
(@TYPE_GO_USE,            1578, 157800,  3000, 5000,     0,     0), -- starts event after 3 - 5 seconds after go use
(@TYPE_CREATURE_ENTRY,   46802, 4680200, 3000, 5000, 10000, 10000), -- starts event after 3 - 5 seconds after the creature id is outfight and repeats the event every 10 seconds
(@TYPE_CREATURE_GUID,   129458, 4680200, 3000, 5000,     0,     0), -- starts event after 3 - 5 seconds after the creature guid is outfight
(@TYPE_QUEST_START,       1044, 104400, 3000, 3000,      0,     0), -- starts event after 3 seconds after quest was accepted
(@TYPE_QUEST_END,         1044, 104400, 3000, 3000,      0,     0), -- starts event after 3 seconds after quest was rewarded
(@TYPE_ON_CREATURE_DEATH, 1438, 143800, 3000, 3000,      0,     0), -- starts event after 3 seconds after creature was killed

DELETE FROM `event_script` WHERE `scriptid`=157800 AND `type`=2;
INSERT INTO `event_script` (`scriptid`, `type`, `delay`, `source_target_type`, `source_target_param1`, `source_target_param2`, `source_target_param3`, `action`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `targettype`, `targettype_param1`, `targettype_param2`, `targettype_param3`, `targettype_param4`, `comment`) VALUES 
(157800, 2, 0, 10, 45879, 1547, 11, 1908, 0, 0, 10, 46802, 129458, 0, 0, 'Event on Go Use - Creatureguid 45879 with id: 1547 - Cast spell 1908 on creature guid 129458 - after 2 seconds'),
(157800, 4, 0, 10, 45879, 1547, 11, 1908, 0, 0, 10, 46805, 129460, 0, 0, 'Event on Go Use - Creatureguid 45879 with id: 1547 - Cast spell 1908 on creature guid 129460 - after 2 seconds'),
(157800, 6, 0, 110, 0, 0, 0, 4680200, 0, 0, 0, 0, 0, 0, 0, 'Event on Go Use - Call spell script 4680200 - after 2 seconds');

Branch(es): 3.3.5 / master (Tell us which branch(es) this issue affects.)
3.3.5 master

TC rev. hash/commit: fe999bc

Operating system: win 10

@BAndysc
Copy link
Contributor

BAndysc commented Jan 17, 2017

What is difference between this and SAI with timed action list? I see source next to the target in event_script, but it can also be added to smart scripts.

@LuigiElleBalotta
Copy link

I remember that with the passage from Cata to Wod there was in consideration the idea to implement some LUA script to the core... wouldn't that be useful for this kind of issue?

@Rushor
Copy link
Contributor Author

Rushor commented Jan 17, 2017

@LuigiElleBalotta no lua is not handy enough for noobs (you can discuss this in irc with the tc member)

@BAndysc: the difference is that sai always needs a reference (creature or gameobject) to handle the script.

example 1: a player completes a quest on npc a. npc a starts an actionlist with some eventstuff. if someone kills npc a the actionlist will be interrupted while on blizz the event will still be excecuted.

example 2: with sai you only have a very limited option two cross handle actions from different npcs.
take a creature a and start an actionlist, where creature a changes the orientation. now creature b (close to creature a) should change the orientation to creature a. currently we need to setdata xy to creature b and then creature b on data xy will change the orientation. Thats issue also concerns the move_to_position action.

In general we need the option that one creature can controle several action for an other creature.

example 3: the guidscript does not work with the entryscript. take a creatureid and assign an entryscript. now take a guid (with the same id) and assign a script. The guid script will always override the entryscript.
With the new event_script_linking_template both scripts can be executed.

@streetrat
Copy link
Contributor

streetrat commented Jan 17, 2017

A Script should be a separate object (lets call it ScriptObject), with a unique id (call it ScriptID) in “scripts” table.
The old TimedActionLists (“script9”) should be removed from “scripts” table and only the basic ScriptObject should be in “scripts” without any ScriptObjectType(sourcetype).
Targets should be an object (lets call it simply Target) in “targets” table with a unique ID for them (ID, Type, Param1-2-3, x, y, z, o)
"scripts" table should contain ScriptID, LineID, basic event/action pairs, targetID_a, targetID_b
TimedActionLists should be moved to "timedactionlists" table with a unique ListID. (ListID, LineID, Timer, ScriptID) where ScriptID is the script to run (which will only use the action+targets)
For example CallRandomTimedActionList would now point to a "timedactionlists"'s ListID
New action should be added: TimeAction(ScriptID, timer-params)
New action should be added: TimeRandomAction(ListID, timer-params)
New action should be added: ReplaceScript(ScriptID, bool: reset to default on Reset/Evade)
All timed actions should be implemented on map-level in core, instead of current object-level (so timers can run after the caller died/removed)
Waypoints should have an orientation field added in “waypoints” table and all paths should have a unique PathId (PathId, PointId, X, Y, Z, O)
New event should be added: OnPathEnd - fires when the the last waypoint is reached from a path
Script-linking should be done in "scriptlinks" table: Entry, Guid, ScriptObjectType(sourcetype), ScriptID
Conditions table should get a unique ConditionID, ElseGroup should be removed
New table "conditiongroups" should be added: GroupID, ConditionIDorGroupReference
New table "conditiongrouprelations" should be added: GroupID, Relation (== !=)

@streetrat
Copy link
Contributor

in core, sai scripts should get a new AI variable, and that in a list or map. this way it would be possible to:
1: load multiple sai scripts to one creature
2: guid scripts would simply work together with the entry based scripts, no more overwritings
3: the sai script can run parallel with any other script on the mob, for example 1 entry based sai + 1 guid based sai + cpp boss script

@Rushor
Copy link
Contributor Author

Rushor commented Jan 18, 2017

so we would have 3 new tables?

"scripts", "timedactionlists" and “targets”

example.

We will add a script where wotlk.openwow.com/npc=1515 starts an event, where http://wotlk.openwow.com/npc=1745 and http://wotlk.openwow.com/npc=10055 should change their orientation to wotlk.openwow.com/npc=1515 with 2 seconds delay.

we add first a targetentry in “targets”

SET @TARGETID   := 600;
SET @TARGETID_2 := 601;
SET @TARGETID_3 := 602;
SET @TARGETTYPE_CREATURE_GUID := 10;
DELETE FROM `targets` WHERE targetid IN (@TARGETID, @TARGETID_2, @TARGETID_3);
INSERT INTO `targets` (targetid, targettype, `entry`, `guid`) VALUES
(@TARGETID, @TARGETTYPE_CREATURE_GUID, 1745, 29777),
(@TARGETID_2, @TARGETTYPE_CREATURE_GUID, 10055, 28478),
(@TARGETID_3, @TARGETTYPE_CREATURE_GUID, 1515, 29797);

then we add the scripts:

SET @ScriptID   := 10000; 
SET @ScriptID_2 := 10001; 
SET @ACTION_CHANGE_ORIENTATION := 69;
DELETE FROM `scripts` WHERE targetid IN (@ScriptID,@ScriptID_2);
INSERT INTO `scripts` (scriptid, actiontype, `target_a`, `target_b`) VALUES
(@ScriptID, @ACTION_CHANGE_ORIENTATION, @TARGETID, @TARGETID_3),
(@ScriptID_2, @ACTION_CHANGE_ORIENTATION, @TARGETID_2, @TARGETID_3);

target_a is here the unit that should do the action ON target_b.

and then we need to add those 2 scripts to the "timedactionlists" table

SET @ACTION_LIST_SCRIPT := 333;
DELETE FROM `timedactionlists` WHERE targetid IN (@ACTION_LIST_SCRIPT);
INSERT INTO `timedactionlists` (action_list_scriptid, delay, `scripts_id`) VALUES
(@ACTION_LIST_SCRIPT, 2000, @ScriptID),
(@ACTION_LIST_SCRIPT, 2000, @ScriptID_2);

@Rushor
Copy link
Contributor Author

Rushor commented Jan 18, 2017

@streetrat

but then I'm still missing the part where timedactionlists @ACTION_LIST_SCRIPT is called. you said the scriptstart would be in TimeAction() but then again we would need to add a script to npc 1515 which will do the action TimeAction(@ACTION_LIST_SCRIPT, timer) on spawn/on death/whatever

example: if several different guids(with different entries) should call @ACTION_LIST_SCRIPT we would still have to add a -guid script for all of them that calls TimeAction() on spawn/on death.. then?

ah nvm, you will link them in "scriptlinks" table. thats a bit confusing, because scripts should not contain any eventtypes, but instead only action and target. i think "scriptlinks" table should be updated then with the "eventtype/source" that should trigger the scripts. maybe it should be done like event_script_linking_template what if posted in the beginning.

@Rushor
Copy link
Contributor Author

Rushor commented Jan 19, 2017

i think "timedactionlists" and "scripts" could be merged in one table since 99 % of those scripts have a timed delay. moreover “targets” table could be moved directly in the "scripts" table. but the table that triggeres the scripts should have the option to add the eventtype, entry, guid and delay + a bool to check if the script should be repeated after it's finished (for waypoint scripts then)

sth. like that:

DELETE FROM `scriptlinks`;
INSERT INTO `scriptlinks` (`script_type`, `source`, `scriptid`, `repeat`) VALUES 
(@TYPE_ON_SPELL_HIT_UNIT,       46802, 4680200, 0), -- starts event after spellhit
(@TYPE_ON_GO_USE,                1578, 157800,  0), -- starts event after go use
(@TYPE_CREATURE_ENTRY_OOC,      46802, 4680200, 0), -- starts event after the creature id is outfight 
(@TYPE_CREATURE_GUID_OOC,       29458, 2945800, 1), -- starts event after the creature guid is outfight, repeat after script is finished
(@TYPE_ON_QUEST_START,           1044, 104400,  0), -- starts event after quest was accepted
(@TYPE_ON_QUEST_END,             1044, 104400,  0), -- starts event quest was rewarded
(@TYPE_ON_CREATURE_ENTRY_DEATH,  1438, 143800,  0), -- starts event creature entry was killed
(@TYPE_ON_CREATURE_GUID_DEATH,   1578, 157800,  0), -- starts event after creature guid was killed
(@TYPE_ON_GOSSIP_SELECT,         4578, 457800,  0), -- starts event after gossip was selected
(@TYPE_ON_GAMEOBJECT_ENTRY_OOC, 14578, 1457800, 0), -- starts event after gameobject entry spawn
(@TYPE_ON_GAMEOBJECT_GUID_OOC,  14578, 1457800, 0), -- starts event after gameobject spawn

DELETE FROM `script` WHERE `scriptid`=104400 AND `type`=@TYPE_ON_QUEST_START;
INSERT INTO `script` (`scriptid`, `type`, `delay`, `source_target_type`, `source_target_param1`, `source_target_param2`, `source_target_param3`, `action`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `targettype`, `targettype_param1`, `targettype_param2`, `targettype_param3`, `targettype_param4`, `comment`) VALUES 
-- Quest example
(104400, @TYPE_ON_QUEST_START, 2000, 10, 45879, 1547, 11, 1908, 0, 0, 10, 46802, 129458, 0, 0, 'Source: Creatureguid 45879 with id: 1547 - Cast spell 1908  - on  Target creature guid 129458 - after 2 seconds'),
(104400, @TYPE_ON_QUEST_START, 2000, 10, 45879, 1547, 11, 1908, 0, 0, 10, 46805, 129460, 0, 0, 'Source: Creatureguid 45879 with id: 1547 - Cast spell 1908  - on  Target creature guid 129460 - after 2 seconds'),
-- Pathing Example
DELETE FROM `script` WHERE `scriptid`=2945800 AND `type`=@TYPE_CREATURE_GUID_OOC;
INSERT INTO `script` (`scriptid`, `type`, `delay_min`, `delay_max`, `source_target_type`, `source_target_param1`, `source_target_param2`, `source_target_param3`, `action`, `action_param1`, `action_param2`, `action_param3`, `action_param4`, `targettype`, `targettype_param1`, `targettype_param2`, `targettype_param3`, `targettype_param4`, `comment`) VALUES 
(2945800, @TYPE_CREATURE_GUID_OOC, 2000, 2000, 1, 0, 0, 53, 2945800, 0, 0, 1, 0, 0, 0, 0, 'Source Self - Load Path Action 2945800 - on Target Self - after 2 seconds'),
(2945800, @TYPE_CREATURE_GUID_OOC, 2000, 2000, 1, 0, 0, 11, 1908, 0, 0, 10, 46805, 129460, 0, 0, 'Source Self - Cast spell 1908 - on Target creature guid 129460 - after 2 seconds'),
(2945800, @TYPE_CREATURE_GUID_OOC, 2000, 2000, 1, 0, 0, 17, 69, 0, 0, 1, 0, 0, 0, 0, 'Source Self - Set Emotestate 69 - on Target Self - after 2 seconds'),
(2945800, @TYPE_CREATURE_GUID_OOC, 2000, 2000, 1, 0, 0, 17, 0, 0, 0, 1, 0, 0, 0, 0, 'Source Self - Set Emotestate 0 - on Target Self - after 2 seconds'),
(2945800, @TYPE_CREATURE_GUID_OOC, 6000, 8000, 1, 0, 0, 53, 2945801, 0, 0, 1, 0, 0, 0, 0, 'Source Self - Load Path Action 2945801 - on Target Self - after random 6 seconds till 8 seconds');

-- this means action 53 starts path 2945800. ALL action BETWEEN "53, 2945800" AND "53, 2945801" should ONLY be triggered once path 2945800 reached it's last point.
-- then after the last action was done, action "53, 2945801" should be started and the creature will load path 2945801. 
-- Once Path 2945801 was finsihed it will repeat everything because `repeat` in scriptlinks is "true".

more ideas by malcrom http://pastebin.com/B3CeFYja

@Rushor
Copy link
Contributor Author

Rushor commented Jan 19, 2017

oh and maybe add a type for spellevents and spellscripts itself (we could remove table spell_scripts, event_scripts and waypoint_scripts then)

@joshwhedon
Copy link
Contributor

joshwhedon commented Jan 20, 2017

Wouldn't this make everything harder to understand and debug ? I mean, your scripting system looks exactly like SmartAIs, but with multiple tables, and exactly the same actions / events as SAIs.
This means 2 things. First, we will have mobs potentially with both SAIs and this script system. When something goes wrong, you got an added complexity (plus the fact that you need to go through multiple tables just for this new system). It also means that some scripts will be in SAIs, others in this new system (and if you think of transitionning to this new system, remember how many years it took to get rid of creature_ai_scripts completely).
Then there is the matter of maintaining it. If the way an event or action is handled changes, it means having to change it in two places, or having to refactor the whole smart ai system to be able to reuse the same code.
Wouldn't extending the smart AI system to cater for special cases be better on the long haul ?

@Rushor
Copy link
Contributor Author

Rushor commented Jan 21, 2017

no both system should work/and must work together. the new system is mostly for out-of-combat actions (waypoint scripts, spell scripts, event scripts). you should be able to assign the new system (actionlist) to the creature in the creature-table, without using a -guid sai script which overrides the entry sai script of the creature.

Afaik, Rat will extend the sai function to make this work.

@streetrat
Copy link
Contributor

creature table wont be good after a few weeks when you realize that you want to assign multiple lists to same creature based on X event or something

@Rushor
Copy link
Contributor Author

Rushor commented Feb 23, 2017

some progress here:

https://github.com/dalaranwow/TrinityCore/commit/f2380a1e95d036cbd5eea8b0ed0e4c3163c97a89

and an example how we could use it:

https://github.com/dalaranwow/TrinityCore/commit/4f6e440be9cdef3a52f99094ae990a884fa3a1b4

without that we would need 3 times more sql for the "Bitter Departure" quest because we had to copy every single -guid script line over and over

@offl
Copy link
Contributor

offl commented Jan 3, 2019

It's pretty similar to CMaNGOS dbscripts. Half of them were removed long time ago from TC and replaced by SAI
Perhaps #22461 will handle anything related to scripts in one system and all issues mentioned here will be fixed?

@offl
Copy link
Contributor

offl commented Jun 28, 2020

Since #22461 was closed and I doubt it will be implemented soon, imho the only problem with SAI is we need possibility to use mechanism of currenct action 'Cross Cast' for all actions.

@BAndysc
Copy link
Contributor

BAndysc commented Jun 28, 2020

The best way to implement "cross cast" like actions is to introduce "source" of action next to "target" of action.

Currently each action has only target, but in fact very often the "target" is actually the "source".

If we had separate columns for "source" (which would be identical to "target"), then any action could have arbitrary source and arbitrary target.

@offl
Copy link
Contributor

offl commented Jun 23, 2022

With all new info all of that looks not good at all

@offl offl closed this as completed Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants