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] new luautil function: getEntitiesInRange() #5816

Merged
merged 1 commit into from
May 27, 2024

Conversation

MowFord
Copy link
Contributor

@MowFord MowFord commented May 22, 2024

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

Sparked by this comment, I've created a more generic binding for finding entities in range of an entity, using the existing CTargetFind class.

I'm putting this in draft mode while I test it, but preliminary testing seems to work (the benefits of using pre-existing systems)

This PR lays some groundwork:

  • Creates a missing enum essential to skills/abilities: AOE_TYPE
  • Copies all targetfind enums into lua space, with some comments about their caveats

The PR's main change: getEntitiesInRange()

  • This binding first does basic checks and returns an empty table if anything essential is missing with the parameters
  • It then calls a PAI->Targetfind flow similar to spells or mobskills
  • the return is always a table, empty if no targets were found within the given targetfind parameters

Steps to test these changes

@MowFord
Copy link
Contributor Author

MowFord commented May 22, 2024

targetfind is funny about conals in that it always adds the primary target my work in #5745 will make it more reliable if one needed to use this binding for raw conal findings, but in general the binding seems to work well.

It's written explicitly to be used for Dark Ixion, so I'm going to start poking at it there

@MowFord MowFord marked this pull request as ready for review May 22, 2024 15:55
@MowFord
Copy link
Contributor Author

MowFord commented May 22, 2024

I've done some testing and it seems to do what's on the tin here. Ixion functions with the new binding, but will benefit greatly from the extra TARGETTYPE in #5745

sol::table CLuaBaseEntity::getEntitiesInRange(CLuaBaseEntity* PLuaEntityTarget, sol::variadic_args va)
{
auto* baseEntity = dynamic_cast<CBattleEntity*>(m_PBaseEntity);
auto players = lua.create_table();
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is targetfind-based, it can find more than players depending on the flags, no?

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 catch, yes. Started from my old binding. I'll update the variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

void recalculateAbilitiesTable();
void recalculateSkillsTable();
void recalculateAbilitiesTable();
sol::table getEntitiesInRange(CLuaBaseEntity* PLuaEntityTarget, sol::variadic_args va);
Copy link
Contributor

Choose a reason for hiding this comment

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

auto getEntitiesInRange(CLuaBaseEntity* PLuaEntityTarget, sol::variadic_args va) -> sol::table;

* Function: getEntitiesInRange()
* Purpose : Returns a Lua table of entities within range of the base entity
* Example : local players = npc:getEntitiesInRange(target, aoeType, radiusOrigin, distance, findFlags, validTargets)
* Notes : if the passed distance is nil or 0 (as well as many other combinations), empty table will be returned
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add a big fat note here, saying something to the tune of: "APART FROM THIS ONE EXCEPTION, ALL TARGET AND PATH FINDING SHOULD BE DONE EXCLUSIVELY IN CORE"

Copy link
Contributor

Choose a reason for hiding this comment

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

and say that this is used for Ixion, and possibly there's another usage for this in assault? I don't remember

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do

Copy link
Contributor

Choose a reason for hiding this comment

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

And another sub-note: If you have to do expensive operations that are meant for core in Lua, you should be doing them as infrequently as possible: In a mobskill that locks itself off, or anything that isn't on-tick, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the description

get all targets in a sphere around an entity
utilizes targetfind on the entity with parameters for aoe type, validtargets, etc
@zach2good
Copy link
Contributor

For any passers-by: This is logic added using TargetFind. Future optimisations using octrees/spatial hashing will happen INSIDE TargetFind.

@zach2good zach2good merged commit a5c9a54 into LandSandBoat:base May 27, 2024
11 checks passed
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.

2 participants