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

[Pre-WIP] [3.3.5] The AI system redesign #22461

Closed
wants to merge 7 commits into from

Conversation

Treeston
Copy link
Contributor

It's Happening™. Eventually. Keep reading.

So.

Our current AI system micromanages practically everything. From making sure the creature keeps swinging in melee, to attacking a new victim, to updating eventmaps or checking health. It also has a bunch of hooks that get invoked from the outside on the most miniscule of events (spell hit, any damage taken, any movement at all).

It's also ridiculously rigid. Events are timed down to the millisecond (+- one server tick) and will always execute at exactly that time.

Retail.... doesn't do any of that. Retail AI is (mostly) asynchronous, and written in a way where delaying AI callbacks a few ticks doesn't make much of a difference. Their scripts set behavior flags for common behavior (should auto attack, should be unkillable, etc.) and those behavior flags are then handled by the core. The result is a large reduction in script complexity, and the core can also make assumptions about what scripts will or won't do. Scripts become less powerful, which is actually a good thing.

(@xvwyh actually wrote a very nice summary of some more differences over here - you should read that)

This PR is where all of that changes. We're throwing out the old AI system in its entirety.
(Realistic aside: it'll survive as LegacyCreatureAI or something for now, likely with reduced capabilities.)

Let's talk about the replacement. Timetable for actual implementation is, most likely, christmas break, because I'm about to be consumed by the upcoming semester, so there's that. Until then, let's figure out what won't work in the proposed model, and then change the proposed model so it does work.

Here's how I see this working out.

The broad strokes (aka: How Do I Do Things)

It's a callback-based model. The script registers callbacks (functions) to happen upon certain events.

Examples of events are health thresholds and timers. Replicating the current update loop with these is possible, but not the intended usage.

Callbacks can be registered/unregistered on the fly. The idea is that any given callback sets up the next "steps".

For example, if a boss changes phases at 60% and 40%, this would look like this:

  • initial setup:
    • set self unkillable (behavior flag, see further down)
    • setup callback 1 on engage
  • callback 1:
    • setup callback 2 at health threshold 60%
    • setup any necessary periodic callbacks
  • callback 2:
    • cancel any unneeded periodic callbacks (i'm considering a validator model so this isn't needed)
    • setup callback 3 at health threshold 40%
    • setup periodic stuff for phase 2
  • callback 3:
    • same as above
    • set self killable

Not that different from our current scripts, right? The main difference is that all of the handling of callbacks is done core-side, and we don't need to call into the actual script on every tick anymore.

(Aside: Evading would also be core-side, and that'd let us get rid of quite a few hacks. I'm also intending to fully reset the "AI object" (I'm not sure I'd even call it that - "Behavior object"?) when reaching home after an evade.)

Behavior flags (aka: How Do I Do When I'm Not Doing)

Pretty much all of our scripts do the same things. Yes, depending on the boss, there's a few special things (that's why callbacks exist) but overall they all do the same thing. We can handle those things coreside using what I call "behavior flags" that tell the core how the creature should act.

Below is a list of behavior flags I'm currently considering for implementation. Please comment if you can think of things you can't do with these.

The combat flags (aka: How Do I Do When I'm Doing The Do-Do)

  • Auto-engage?
    • true
      • if not engaged, periodically check for nearby targets we can engage
      • (this is the behavior of current REACT_AGGRESSIVE in base CAI)
    • false
      • never engage on our own, wait for other things to engage us first
  • Auto-attack?
    • true/false - whether we should ever do melee swings
    • even if this is true we might use spells instead depending on priority!
  • Using threat?
    • NO/FIXATE/YES - whether we should care about the threat list
    • if this is set to NO or FIXATE, the core will never override the victim selection
    • if this is set to FIXATE and current victim goes away, snaps back to YES
  • Can evade?
    • true/false - what it says on the tin; whether we should evade if there's no targets to punch anymore
    • Is this even necessary? Feels like the previous flag might handle this just fine.

The interaction flags (aka: How The Do-Do Is Done To Me)

  • Can I take damage?
    • true/false
  • Unkillable?
    • true/false
    • Will remain at 1 HP without dying if true is set - bosses should set this outside their final phase

Spell priority system (aka: I Do The Do-Do Without Any Do!)

  • Inspired by PlayerAI
  • Creature has a list of spells it can cast
    • Each spell comes with properties (priority, "cooldown", target selection...)
    • Kind of like SAI, just flexible!
  • This is for non-boss mobs that should be even more dynamic
  • Some randomness in the selection of spell to cast

Some more thoughts on callbacks (aka: How Do I Do It Do the Doing)

I'm envisioning three different kinds of callbacks that can be used more-or-less interchangably:

  • "Regular" C++ callback
    • I am quite tempted to restrain what these can do, but it may be hard
    • Restraining can bite you, we might need the flexibility
    • With Great Power etc
  • Lua callback (IT'S HAPPENING)
    • Because we only go into the callback rarely, we can afford the overhead now!
    • Lua is sandboxed, so less possibility for hacks 😊
      • Relevant behavior manipulation functions exposed in the sandbox
      • Core itself isn't exposed - because callbacks shouldn't touch the core! Bad!
  • DB-based callback
    • Kind of like current SAI already works!
    • Mostly for stuff like quest credit
    • "Native" spell casts should be handled by spell priority system!

Callbacks don't have to be real time, and shouldn't depend on this! If a callback happens to happen a second or two late, this shouldn't break anything.

(Aura scripts are for precise stuff. Those are staying as is.)

What I Want You To Do(-Do)

Think about scripts you've worked on. What things do they do that doesn't cleanly fit the model above? Bring it up. I would prefer to find out now rather than halfway into the implementation.

What things would you like it to do it currently doesn't? Do you think some things could be more streamlined? More input on how you've found retail does things? I'm interested.

Like anything above a bunch? Tell me, so I can make sure not to cut it if possible. This is a draft after all.

PS: I am truly a bit of a masochist, I suspect.

@Scytheria23
Copy link

A great proposal. What would make it really powerful is the ability to make templates that can be applied to more than one creature with level scalability. For example, if a template is made for a 'Frost Mage NPC', that template could be applied to innumerable different NPCs of different levels. Does that make sense?

@Treeston
Copy link
Contributor Author

Treeston commented Sep 18, 2018

Kind of @Scytheria23, but what would this template consist of? If it's just the frostbolt spell, what's the point in having a template?

(Also, different creatures use different Frostbolt spells...)

@sirikfoll
Copy link
Contributor

Some behavior I saw on 3.3.5 sniffs of kologarn, there were a "cast sequence", measured in seconds passed from fight start:
18s - Casts 'Arm Sweep'
28s - Casts 'Overhead Smash' (Failed due to category cooldown)
29s - Casts overhead Smash
34s - Casts Stone Grip
Those spells share category in dbc, with the cooldowns as follow: Arm Sweep(10000ms), Overhead(4000ms), Stone Grip(8000ms)
I would expect those cooldowns to be accounted for, although it seems they were not, anyways, spell category cooldowns probably needs to be considered in the new system

@Treeston
Copy link
Contributor Author

Treeston commented Sep 18, 2018

Hrm, a success/failure result for callbacks, perhaps?

ED: Upon further thought, feels like an overcomplication. Callback can just reschedule itself if necessary.
ED2: The "basic" callbacks (cast X spell at Y target) will need options for how to handle failure, though.
ED3: Also, a way to chain basic callbacks together (actionlist-like?)

@xvwyh
Copy link
Contributor

xvwyh commented Sep 18, 2018

Since I was pinged here...
I currently don't have time to read this entire thread, but later (in the coming days) I will do a writeup about everything I managed to gather so far about Blizzard's own scripting sytem. They do something similar to SAI's actionlists, but with waitable actions (for example "Idle for 5 seconds and move to next action"), but apparently also LUA? (So far I'm inclined to believe it's only used for spell scripts, but I'm probably wrong). IMO if you decided to design/redesign yet another AI system, might as well try do it the way Blizzard does it.

Oh and all the info about their "actions" is in the client, I extracted it from a few builds (see #22421 for an example) and will post everything I have.

@xvwyh
Copy link
Contributor

xvwyh commented Sep 18, 2018

Alright, read it.
Seems like a logical evolution of TaskScheduler. I wonder if it won't turn into callback hell, though.
But still. Ideally scripts should only be data. No C++ code, maybe some C++/Lua code for spells, optional Lua code for enhancing scripts (a-la SPELL_EFFECT_SCRIPT_EFFECT, but for scripts). When you start mixing behavior in, then the core is no longer solely responsible for handling scripts, and thus can no longer account for everything.

As for "Behavior flags", "combat flags" and "interaction flags", once I post the aforementioned list of actions dug out from the client, it will be very clear which flags and toggles you need to have, anything above that would only need to be added to account for whatever quirks the new system will have (ultimately none, hopefully).

Also, adding to what I wrote above, if you decide to mimic Blizzard's scripting system, there are three main worries I see:

  1. Whether we have enough info to figure out completely how it's supposed to work. Gaps in knowledge can be plugged with custom decisions, but it will forever remain unknown if said decisions won't put us on a wrong path and lead us astray.
  2. Whether it will even be feasible to develop such scripts without proper editors. SAI is tedious enough already, if you don't use any of the available editors (none of which, let's face it, are ideal) and juggle with numbers in DB manually. Blizzard's system, while similar, will add more complications in terms of interchangeable events (triggersets) and spells (CreatureSpellData, tables with spells and timers on when to use them), without some sort of grouping system it will become easy to get lost in them, and devs often really dislike leaving meaningful (and standardized) comments.
  3. Whether (and when) the system will get developed enough to replicate all the existing boss scripts in it while continuing to look like retail.

@funjoker
Copy link
Member

Do not forget master

@Warpten
Copy link
Contributor

Warpten commented Sep 19, 2018

(Aside: Evading would also be core-side, and that'd let us get rid of quite a few hacks. I'm also intending to fully reset the "AI object" (I'm not sure I'd even call it that - "Behavior object"?) when reaching home after an evade.)

Keep in mind that some (most now?) bosses do not evade on party wipe, but despawn and respawn after a fixed timer.

if this is set to FIXATE and current victim goes away, snaps back to YES

A good check for this behavior is yors'ahj in DS, adds during the black phase are set to fixate, what do they do when their target dies? (I think they used to ignore feign death too)

I also share concerns about callback hell. Tracing call stacks when bogus situations happen is also going to become a lot more difficult.

Re LUA: Do you allow coroutines?

They use some sort of SAI tool indeed (It's integrated into WowEdit, and they leaked it with a screenshot of the first Onyxia - at work, but the screenshot is out there somewhere), and some NPCs in that gun'drak dungeon with the optional dinosaur boss.

@Treeston
Copy link
Contributor Author

Treeston commented Sep 19, 2018

@xvwyh:

When you start mixing behavior in, then the core is no longer solely responsible for handling scripts, and thus can no longer account for everything.

Yeah, that's kind of what I meant when I said "restrict what they could do". I want to keep allowing C++ callbacks for extreme cases (also, for more efficient built-in callbacks) though. 99% of scripts should be doable in just Lua+DB.

@xvwyh:

As for "Behavior flags", "combat flags" and "interaction flags", once I post the aforementioned list of actions dug out from the client, it will be very clear which flags and toggles you need to have, anything above that would only need to be added to account for whatever quirks the new system will have (ultimately none, hopefully).

Yes please.

@Warpten:

Keep in mind that some (most now?) bosses do not evade on party wipe, but despawn and respawn after a fixed timer.

Yeah, what I mean is evade and respawn would (in terms of behavior object) do exactly the same thing.

@Warpten:

Re LUA: Do you allow coroutines?

Ehhhh. Maybe. I'm not sure if there'd be any significant upsides to it.

@Naios
Copy link
Contributor

Naios commented Sep 19, 2018

In my opinion this issue should be seperated into two main problems:

  • Unnesseccary calls to UpdateAI where it should be known when the AI is updated next (at the next auto attack or occuring event). I admit that there is overhead for the EventMap as well as the function calls for each AI, however, this overhead has not shown to be problematic in the past. Actually one should performance measure how much time is spent in UpdateAI relative to a whole world update.
  • Really complicated and difficult to use C++ scripting API.

I did some research on this topic in the past (from which the TaskScheduler and the hotswap system originated from) and for me I came to the conslusions that:

  • C++ could be highly suitable for scripting in case we provide a new C++ API for scipting:
    • Callbacks don't neccessarily mean that we will encounter a callback hell, we could wrap them nicely into promises and futures - self promotion warning. However for our use case they will lead to a frequent memory allocation for the type erasure, the impact on this also relies on the underlying executor we will use since boost asio erases the callbacks by default.
    • Fibers (stackful coroutines) are a much better option here, we could allocate small stacks a 10kb for scripted tasks that can be suspended and canceled at any time which would make it easy to embed suspendable actions even in existing scripts. We could wrap them also nicely as promises and futures that are not shared across cores which makes it possible to implement them nicely with cross referenced pointers.
    • Stackless (compiler supported coroutines) the support for this seems not good enough currently and also those could lead to inefficient memory allocation.
  • A scripting language would also be an option however I highly vote for JavaScript (v8 engine) over Lua since there is much more effort put into this nowadays (Typescript for instance).

I did some experiments with boost fibers int the past actually I got the following example to work. It is similar to the design proposed by @Treeston with a single entry point:

class MyAI : public AsyncCreatureAI
{
  public:
    void Reset() override
    {
        await OnEnterCombat();

        Async([this] {
            await OnDespawn();

            await this->CastSpell(47474);
        });

        Async([this] {
            while (auto damage = await OnDamageReceived())
            {
                // Do something
            }
        });

        while (true)
        {
            RandomEvent(
                [this] {
                    await Wait(10s, 15s);
                    await CastSpell(28373);
                },
                [this] {
                    await Wait(8s, 13s);

                    while ((await CastSpell(3746)) != SpellCastResult::Ok)
                        ;
                });
        }

        Async([this] {
            // ...
            await OnDespawn();
        });
    }
};

@ratkosrb
Copy link
Contributor

They use some sort of SAI tool indeed (It's integrated into WowEdit, and they leaked it with a screenshot of the first Onyxia - at work, but the screenshot is out there somewhere), and some NPCs in that gun'drak dungeon with the optional dinosaur boss.

Here are the screenshots of blizzard's tools that @Warpten mentions. They were posted on mmo champion.
Creature Editor - https://static.mmo-champion.com/mmoc/images/news/2009/august/raidsdungeons_020.jpg
Creature Spells - https://static.mmo-champion.com/mmoc/images/news/2009/august/raidsdungeons_021.jpg

You can see that in the Creature Editor, which presumably is for editing blizzard's equivalent of the creature_template table, there is a button for "Spells/Abilities", and Onyxia has the currently selected entry from the Creature Spells editor screenshot assigned to her creature template. In the screenshot of the Creature Spells editor we can also see there are multiple "spell\ability templates" for Onyxia, for her different phases, so presumably the used "spell\ability template" can be changed in combat by the AI script.

@Treeston
Copy link
Contributor Author

Especially the creature spells editor is super interesting.

@ratkosrb
Copy link
Contributor

ratkosrb commented Sep 19, 2018

We can try to deduce the following about the "creature spell lists" from that editor screenshot.

  • Both normal and boss creatures use these "spell/ability" lists.
  • A single creature spell/ability list can contain information for up to 10 spells.
  • The actual use of the abilities is possibly done by the AI script, since there is no option to set the target of the cast or other conditions in the Creature Spells editor.
  • Managing the cooldown of abilities is separate from the AI script, as cooldowns are defined in the creature spell/ability list entry.
  • Cooldowns are defined in whole seconds, not milliseconds.
  • Cooldowns are a range, there's a minimum and a maximum.
  • The initial delay to use an ability is separate from the repeat cooldown.
  • The Availability field seems to be a percent chance, as the default is 100.
  • Creatures can have multiple spell/ability lists, for the different phases of the fight, with one being the default that is assigned to the creature_template entry.
  • The Blizzard convention for naming these spell/ability list entries is:
    <Zone Name> - <Creature Name> <Phase Name>

I have no idea what the Probablity field means, it seems to have a default value of 1. Possibly a mask, specifying the allowed difficulty modes like Normal/Heroic? But the name doesn't fit that.

Up top there is a box titled "Percent chance each tick", which lets you set a "Chance Support Action" and "Chance Ranged Attack". The value "-1" probably means "not applicable".

There is an "Edit" button next to the spell name button, so maybe clicking that brings up a pop up window that lets you set additional data for that spell like some kind of flags. Or maybe it would bring up a "Spell Editor" to edit the spell itself.

The version of the editor that was posted on mmo champion is from 2009, based on the URL, possibly shown at Blizzcon. Here is an earlier screenshot of the tool from vanilla. The early version is missing 3 of the fields on the right, the rest is the same.

@Treeston
Copy link
Contributor Author

I would disagree with some of the points there.

  • The main entry is likely not just a spell ID (note it says Creature - Dragon Tail Sweep (Onyxia) (Tail Sweep), not just `Tail Sweep´) - I would assume that targeting information is contained in there.
  • Probability is... a probability. I'm not sure why you think it's a mask. Range 0~1 inclusive. Probability to actually do the action, maybe? Hard to tell.
  • Availability.... maybe only certain instances of the creature have a spell? Seems unlikely. My guess is as good as yours.
  • Initial/Repeat are almost certainly how often the action happens. This, incidentally, is the same way our SAI repeat timers work.
  • I do not think the "AI script" is actually involved with the first four spells, as outlined above. The fifth spell (Deep Breath) with ???? fields is probably something that's AI triggered.

@xvwyh
Copy link
Contributor

xvwyh commented Sep 19, 2018

CreatureSpellData.dbc, hint-hint.
Sadly Blizzard has hid all the info from us except for those "spells records" used by pets.

@xvwyh
Copy link
Contributor

xvwyh commented Sep 19, 2018

I do not think the "AI script" is actually involved with the first four spells, as outlined above. The fifth spell (Deep Breath) with ???? fields is probably something that's AI triggered.

No, that's just a joke on players, who were always confused about the timing of that particular Onyxia attack. Blizzard teased them by showing a screenshot of Onyxia's spells, but deliberately hiding the Deep Breath values.

@xvwyh
Copy link
Contributor

xvwyh commented Sep 19, 2018

The main entry is likely not just a spell ID (note it says Creature - Dragon Tail Sweep (Onyxia) (Tail Sweep), not just `Tail Sweep´) - I would assume that targeting information is contained in there.

Nah, it IS spell ID, but blizzard has internal names for everything. You think any of the designers would be happy to work with 200 spells named "Fireball"? Even the creature editor there shows a field with "Internal Name".

@xvwyh
Copy link
Contributor

xvwyh commented Sep 19, 2018

I haven't even begun writing the theoretical part yet (it will contain my thoughts on WowEdit screenshots), but I've assembled the scripting actions for you to get started:

https://docs.google.com/spreadsheets/d/1Wh1e1ZXYM-sw3wWrY9dGZ2ngK8oa3PA2XIftQJuf5R0/edit?usp=sharing

If you don't understand the meaning of some of the things in there, there's a high chance I will explain them in my writeup, but feel free to ask (or comment on the document) anyway.

@ratkosrb
Copy link
Contributor

  • Probability is... a probability. I'm not sure why you think it's a mask. Range 0~1 inclusive. Probability to actually do the action, maybe? Hard to tell.
  • Availability.... maybe only certain instances of the creature have a spell? Seems unlikely. My guess is as good as yours.

If the values for Probability and Availability were reversed, i would think that too. Probability is chance to cast, and Availability is possibly a mask for difficulty. But instead you can see that Availability has a value of 100 for all the spells, which seems like percent chance to me, while Probability is 1. The ??? on Deep Breath is a joke for sure btw.

Also i just noticed, from that vanilla screenshot we can also see that the "creature spell lists" contain self buffs - "Demon Armor III" is highlighted.

@Treeston
Copy link
Contributor Author

No, that's just a joke on players, who were always confused about the timing of that particular Onyxia attack. Blizzard teased them by showing a screenshot of Onyxia's spells, but deliberately hiding the Deep Breath values.

I feel very stupid right now.

@ratkosrb
Copy link
Contributor

This is where the screenshots are taken from if you want to watch the panel.
https://youtu.be/nt8BffVACQE?t=2h10m5s

@Scytheria23
Copy link

What would make it really powerful is the ability to make templates that can be applied to more than one creature with level scalability. For example, if a template is made for a 'Frost Mage NPC', that template could be applied to innumerable different NPCs of different levels.

Well, I'm thinking for coverage beyond Blizz-like, which I know isn't TCs objective but could be considered. Many devs have quite complex custom NCP behaviours scripted with smartAI, but these have to be individually scripted for each creature type. A more generic template could be applied to a range of NPC, with abilities scaling by level.

@Warpten
Copy link
Contributor

Warpten commented Sep 19, 2018

These strings look horribly familiar and I don't know why. I remember finding them in some IDB and casting it aside as "probably debug shit".

IMHO if youre going with Lua you wouldn't allow coroutines there, because that's more or less just moving the overhead somewhere else

@xvwyh
Copy link
Contributor

xvwyh commented Sep 19, 2018

They are in every Wow.exe. They are probably located in some source code that's shared between client, server and WowEdit (e.g. our SharedDefines.h), and thus get compiled into the exe, even though they're never used anywhere by the client.

switch (stepTemplate.StepType)
{
case ACTIONSCRIPT_NULL:
step = new NullStep(thread, stepTemplate);
Copy link
Contributor

Choose a reason for hiding this comment

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

If "steps" are anything like SAI's actions or even events, dynamic allocations every time a script is going to execute an action (or event) seems very detrimental to performance. Consider using std::variant, so no allocations will need to be done and all possible steps can live within the same already-allocated memory. Although it will probably make it very inconvenient to declare said variant, since you would need to list all possible step implementations in it. Another way would be to have a pre-allocated memory arena that will host all steps (with "placement new"s), however that would require defining strict limitations on step sizes (nothing a static_assert can't handle) to make sure they can even fit inside the arena.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good observations. Variant would be a complete pain to work with.

Looking into placement new into a pre-allocated region in the ActionThread.

ActionThreadStep(ActionThread const& thread, ActionScriptStep const& stepTemplate) : _thread(thread), _template(stepTemplate) {}

ActionThread const& _thread;
ActionScriptStep const& _template;
Copy link
Contributor

@xvwyh xvwyh Dec 26, 2018

Choose a reason for hiding this comment

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

I'm sure it sounds logical to have these things as members here, but take a moment to think if it's really necessary. Will programmers have access to steps outside of the context of their execution? If not, these references can easily be passed to steps when needed instead of having them stored here permanently. Have a structure, say, ActionScriptExecutionContext and pass it to every Evaluate (Execute seems to me like a better name?). Said structure will contain references to ActionThread, ActionScriptStep template (since they all seem to have explicit numbering, i'm sure you'll be able to access the list of all step templates and pick the one corresponding to the current step), and additionally things like current time or time delta, current unit, last invoker and whatnot. This way each step instance will only have to physically contain the actual data it requires to run, data, that's going to be unique to each instance of a step. Oh and as a bonus, such temporary context-like parameters tend to get optimized away neatly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Historically, not having access to (something like) the ActionThread has always turned out to be a maintenance nightmare. A single extra pointer is not going to kill us.

ActionScriptStep is only stored here, not in the thread. We need access to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's 2 pointers already which are going to be duplicated on every creature in the world using ActionScript. Well, I guess as long as only the "current" steps are being stored, the memory usage might be within reason, but if at any point it's going to be like SAI, where the entire script is copied into every of its instances because it might need to be changed (SMART_ACTION_CREATE_TIMED_EVENT, SMART_ACTION_INSTALL_AI_TEMPLATE etc), this could blow up.

I still think it's inefficient to store the reference to thread if the step can never be interacted with by anything other than the thread. Store just the state of the step, pass everything else needed for it to function as parameter(s). Unless you intend to do something like unit->GetCurrentStep().Cancel(), which would require the step to have all the necessary information (thread, namely) to function in such independent context, but IMO that's not the best design.

But sure, this kind of optimization can be done when the design is finalized and proven to be working, if it doesn't clash any of the implemented functionality.

Choose a reason for hiding this comment

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

By storing refs you also just implicitly deleted the other special member functions, maybe not too handy for later.

@@ -29,15 +29,16 @@ class NullStep : public ActionThreadStep
void Abort() override {}
};

/*static*/ std::unique_ptr<ActionThreadStep> ActionThreadStep::StepTo(ActionThread const& thread, ActionScriptStep const& stepTemplate)
/*static*/ void ActionThreadStep::StepTo(unsigned char* ptr, ActionThread const& thread, ActionScriptStep const& stepTemplate)
Copy link
Contributor

@xvwyh xvwyh Dec 26, 2018

Choose a reason for hiding this comment

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

Is passing ptr really needed here, if you already have access to the thread? &thread.GetCurrentStep() and voila, no need for additional reinterpret_casts either. Would require thread to not be const, of course.

It might be needed, though, if at any point will you decide to have more than one "current step".

else
return STATE_RUNNING;

step.~ActionThreadStep();
Copy link
Contributor

Choose a reason for hiding this comment

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

it doesn't happen often to see a destruction being called explicitly

switch (stepTemplate.StepType)
{
case ACTIONSCRIPT_NULL:
new(ptr) NullStep(thread, stepTemplate);

Choose a reason for hiding this comment

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

Why placement new for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because memory fragmentation is not a good thing to have in code that runs all the time, and we don't need malloc here.

Choose a reason for hiding this comment

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

I thought TC had a dependency library that allowed static mempools. If not then I'll agree with you to use p-new.

@Treeston
Copy link
Contributor Author

Treeston commented Jan 7, 2019

Alright, so this is gonna be a two-parter comment. This here is the first part, where I talk about some more design considerations I've been making. Part two, probably somewhere below by the time you're reading this, is where I engage in some Real Talk™ and tell you why I'll be closing this for the time being. It's not for the reasons you think.

So, AI design. If you've been following the PR, you will have already noticed the outlines of a SAI replacement taking shape. I called it ActionScript, name subject to change. Maybe ActionSequence would be better. I don't like calling it "Script", it sounds rigid. Maybe it's accurate, though. Either way.

In my vision, this true kinda-synchronous "scripting" system would only be necessary for truly "scripted" passages in a fight. Let's take Lich King, for example - ActionScript would handle the transition phases. Triggered at a previously-set HP threshold by the core, the script would take control, stop auto-attacking, move LK to the center of the platform, start channeling big bad AoE, and start summoning stuff periodically (maybe there's even some aura that does this), wait X seconds, stop channeling, cast "break the platform stuff" spell, end of script.

Here's why - one major problem I keep running into, which I really do not think should be a problem, is the consideration of what happens if multiple "things" try to directly control a given unit at the same time. What if ActionScript tells the creature to move to point A, but the creature behavior wants to chase to point B? You can see how this gets annoying really quickly.

So, none of that. In my vision, only a single "thing" has control of a given unit at any one time. That could be ActionScript, or it could be the creature behavior engine, or something else, but never multiple of these. The creature starts executing an ActionScript, normal creature behavior stops doing anything until you finish. Once you finish, behavior comes back.

Now you will notice that I said "has control" up there. In my vision, there's two ways you can run an ActionScript - I call them "synchronous" and "asynchronous", because the names fit. Synchronously running an ActionScript is what I described above - the script takes control of the creature, and does stuff with it, and normal behavior is paused. Asynchronously running the script lets it run concurrently to normal creature behavior, but you cannot control the unit (this is enforced by the engine). You cannot tell the unit to move to position X, or to cast spell Y, or to stop attacking. You can modify behavior flags or the creature's spell list, though, and the behavior of the creature will change based on that - so you can't directly cast spell Y, but you can add spell Y to the creature's spell list, and the creature will cast the spell whenever behavior deems it appropriate. Stuff like "cast spell Y if condition Z is met" uses this.

Oh, and if that wasn't obvious: no two synchronous ActionScripts can run at the same time. Not sure on the details, but either the first one gets aborted, or the second one gets delayed until the first is done. Tending towards the latter.

With that, we're done with the technical stuff. Real Talk follows in the next comment.

@Treeston
Copy link
Contributor Author

Treeston commented Jan 7, 2019

Alright, part two, the Real Talk™ part.

You may recall that I promised I'd do work on this over the break, and that work has only manifested to a limited degree.

See, and this is where we veer right into Real Talk. We all make commitments sometimes - but there's a point where there are too many, and you are constantly just dealing with the next expectation looming over your head, where this gets really shitty for your mental health.

For some more context - and some of you might be aware of this already - besides being a somewhat prolific contributor here, I'm also a second-in-command for a tiny internet spaceship alliance of a few thousand real people, I routinely obsess over card rulings for pointless card games on reddit, and also manage the full workload of being a student and TA at the same time. Apparently, when I have hobbies, I have them way too much.

So yeah, for the last few years, I would wake up, and be on my phone checking Slack and Discord within the first five minutes of my day. After that, I'd go through my mail and deal with my Github inbox. After that, reading through all the threads about internet spaceships. After that, over to reddit and make sure all the card game rulings had accurate replies. Before I'd know it, it'd be time to leave for work, where you can guess how I spent my breaks. Once I got home, more of the same. Just before heading to bed, I'd be checking Slack, Discord and forums.

Sounds kind of like your daily routine? As it turns out, constantly being under pressure to perform even more is not good for your mental health. Like, at all.

There's this thing called "burnout". Yes, I know, we don't talk about it. It makes us seem less like the perfect, efficient, always functional, well-oiled rational machines we've all been trained to be. If you don't work hard, you're letting other people down and showing you're not worth keeping around, right?

Yeah, fuck that. I spent my "break" still being torn between four different commitments, one of which is writing a new AI system. And, you know what? I still wanted to write a new AI system. Like, consciously, I wanted to. That's why there's the whole comment above with stuff I thought about.

But, mentally? I can't anymore. I'm unable to focus on the work in front of me. In other words, I've got what we'd probably call "Mental Issues™". You know, the things you're not supposed to have, because they make you seem like a less valuable job applicant, and that's all we care about anymore.

Maybe we should all face some truths about us not being all too perfectly functioning all the time sometimes, hm?

Anyway, that's my Real Talk for today. No big lessons or great wisdom in here. Just a plea to watch your own mental state, before you end up where I am right now. I'll get better, I'm sure - but it won't be today, or in a week, or maybe even in the near future.

Right now, more work is not something I am consciously deciding not to deal with, so I'm closing this.

If I ever feel like working on it again, it may come back in the future - which is decidedly not a commitment. If that happens, it'll happen on my terms, because I actually want to work on it, not because I set myself up for yet another commitment that pressures me into meeting expectations.

Mic is dropped. Tree out.

@Treeston Treeston closed this Jan 7, 2019
@jackpoz
Copy link
Contributor

jackpoz commented Jan 7, 2019

Hej, sorry to hear to went through what many others went through already, hope you'll find your balance between work, family and hobbies. It's fun to have hobbies and it's very easy to get totally sucked up into those.

Small tip from a guy who is trying to build a whole automated AI that is supposed to test all possible already fixed TC issues and find new ones on its own: when you have an idea that sounds very interesting to work on, take a note about it, write what you would like (or "dream of") to do about it, just like this AI redesign. Maybe it will take 10 years to finish, maybe it will take 1 year to even start writing the first line of code, it will just be a background thread running when you are bored enough to work on it and not exhausted by everything else. It will allow you to be curious about things while keeping your sanity and having time for your life :)

@Kittnz
Copy link
Contributor

Kittnz commented Jan 7, 2019

Iv found out that sometimes you have to pause stuff and let it sit a for a while and when you feel up to work on something, do it. But don't do it because you promised it. Keep your head above water @Treeston .

@ccrs
Copy link
Contributor

ccrs commented Jan 7, 2019

I really feel you.
Each person is unique, of course, but I'd say that I went through something almost identical last year.
In my opinion, looking at my experiences and results, the decision you've taken is the most wise and mature. As a quick reference, I took the path of tasks/commitments/obligations prioritization and division, and have to say I'm really happy with the results.

Even if we had our differences in opinion in the past, I have to say it was a pleasure debating, discussing and even arguing about literally everything. I really think I've learned from that, hope it was a two-way thing.

Really wish everything goes well for you, I mean it.
Dont hesitate to contact me any time.

Cris.

@ghost
Copy link

ghost commented Jan 7, 2019

Thank you for all the things you already have provided to TC.
I struggle with autism, so I can only imagine the stress you feel.
I wish you all the best in your endeavours, as well as finding a healthier life.

@jameyboor
Copy link

Remember this is all open-source free contributions you're making here. You should never feel pressured to write anything, as you've already contributed a lot. Take some time off @Treeston . Thanks for your work.

@nawuko
Copy link
Contributor

nawuko commented Jan 8, 2019

Hey Treeston, you have done awesome work for the community and seem all arround like a really nice guy. Had to deal with the same issue aswell in the past, and it will get better, but like you said, it takes time. Wish you all the best ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.