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

Added Behaviors Actor sub system #2908

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

Boondorl
Copy link
Contributor

@Boondorl Boondorl commented Jan 23, 2025

These allow for arbitrary behaviors to be attached to Actors in a way that doesn't require the use of Inventory tokens. The list can't be freely modified nor can it have duplicates meaning it has far better deterministic behavior than using Inventory items for this purpose since they can go poof at just about any time.

One of the strengths of these is that you can iterate across all Behaviors and not just across the behaviors of individual Actors. This allows you to easily apply sweeping events/changes across all of them similar to an interface.

Allows for arbitrary behaviors to be attached to Actors in a way that doesn't require the use of Inventory tokens. This list can't be freely modified nor can it have duplicates meaning it has far better deterministic behavior than using Inventory items for this purpose.
@RicardoLuis0
Copy link
Collaborator

would be good to have a playerbehavior subclass (would need to be attachable to any actors because of morphs)

Collect all objects first to account for the map being modified mid-iteration.
@Boondorl
Copy link
Contributor Author

Boondorl commented Jan 23, 2025

Players can only morph into other PlayerPawns so it'll be safe in the regard. Would just need to add a class type check in AddBehavior

@Boondorl
Copy link
Contributor Author

The more I think about this, the more neat I think it'd be to also store a bucket of Actors based on their given behaviors. That way you can also look up a list of Actors by their behaviors similar to a proper interface. Wouldn't let you type check still, but nice for quickly grabbing anything you know could have it.

Renamed Readded to Reinitialize.
Imported Behaviors to the engine to allow them to properly clean up their level list. Restrict Behaviors from being new'd in ZScript as they need an owner to function.
@MajorCooke
Copy link
Contributor

This looks quite promising. Ricardo brings up a good point though, having a player specific one would be helpful because then I could implement the weapon switching events in the other PR I made.

@Boondorl
Copy link
Contributor Author

I think for now I'm going to hold off on adding more virtuals/subtypes since there's some discussion about allowing generic events that can be hooked into rather than using virtuals for everything. I've never been a fan of this pattern because of how inexpressive it is and it might become obsolete in the near future.

Also adds support for foreach loops with the behavior iterator.
@Boondorl Boondorl marked this pull request as ready for review January 25, 2025 16:15
@MajorCooke
Copy link
Contributor

Okay, THAT would be awesome. I look forward to seeing that coming about!

@MajorCooke
Copy link
Contributor

Quick question though. How would we transfer these between levels? Or do they need to be re-applied whenever a level transfer happens?

Not that it matters much and doesn't change my stance at all - I'm eager to give this a try.

@Boondorl
Copy link
Contributor Author

They carry over the same way any object on the player and their items does. The only management that was needed on the backend was properly unlinking and relinking them when traveling since the level list needs to be cleared on load.

@inkoalawetrust
Copy link
Contributor

Question, can the behavior finder functions also find child classes? i.e instead of only looking for MyCustomBehavior, it also returns any behaviors that descend from that behavior class too.

// These don't make sense without an owning Actor so don't allow creating them.
if (cls->IsDescendantOf(NAME_Behavior))
{
ThrowAbortException(X_OTHER, "Behaviors must be added to existing Actors and cannot be created with 'new'");
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't hurt to add "Use 'Actor.AddBehavior' instead" similar to what you added with Actor.Spawn above.

@Boondorl
Copy link
Contributor Author

Boondorl commented Jan 28, 2025

Question, can the behavior finder functions also find child classes? i.e instead of only looking for MyCustomBehavior, it also returns any behaviors that descend from that behavior class too.

I've thought about it but I'm not entirely sure how useful this would be tbh. It might be better to make an iterator for that purpose so you can work on all child classes instead of just the first one found. Tbh it doesn't make a ton of sense on FindInventory either in that regard.

I will be adding some more stuff soon that I feel is missing as I've thought more about the API.

Use TObjPtr to improve null checking. Add option to clear only specific kinds of Behaviors. Fixed removal checks while ticking. Don't stop at now-null behaviors when iterating.
Allows for proper cleaning up of behaviors when moving between Actors. Important for player respawn handling and morphing.
@inkoalawetrust
Copy link
Contributor

Question, can the behavior finder functions also find child classes? i.e instead of only looking for MyCustomBehavior, it also returns any behaviors that descend from that behavior class too.

I've thought about it but I'm not entirely sure how useful this would be tbh. It might be better to make an iterator for that purpose so you can work on all child classes instead of just the first one found. Tbh it doesn't make a ton of sense on FindInventory either in that regard.

I will be adding some more stuff soon that I feel is missing as I've thought more about the API.

I think it could be useful for more generic behaviors, for example one use case I can already think of for behaviors is adding an optional independent multi-targeting system to KAI that stores and manages multiple targets. But someone might want to make a different variant of the system for their NPCs, i.e one that doesn't just sort the target array on the behavior by most dangerous, but instead something too esoteric for me to add to the default system. Yet it would still be useful if mods could detect if a KAI_Actor has any form of MTS by also being able to check for any behaviors on the actor being checked that descend from KAI_MultiTargetSystem, not just if the stock system is present (Or having to make hardcoded checks for only a certain subset of child classes, i.e whatever variant the mod doing the checking uses + the built-in one).

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

Successfully merging this pull request may close these issues.

4 participants