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

[Request] Add TaskSequence abstractions. #51

Open
lfshr opened this issue Feb 2, 2021 · 8 comments
Open

[Request] Add TaskSequence abstractions. #51

lfshr opened this issue Feb 2, 2021 · 8 comments
Labels
enhancement New feature or request

Comments

@lfshr
Copy link
Contributor

lfshr commented Feb 2, 2021

Is your feature request related to a problem? Please describe.
Add equivalent of C# TaskSequence class (https://github.com/citizenfx/fivem/blob/747e5a4c9645726bfb5f283321218d80304692e3/code/client/clrcore/External/Tasks.cs#L472-L552)

Describe the solution you'd like

const sequence = new TaskSequence();
sequence.AddTask.GoTo(position);
sequence.AddTask.GuardCurrentPosition();
sequence.Close();
somePed.Task.PerformSequence(sequence);

Additional context
This is more of a discussion issue before I start typing things.

@lfshr lfshr added the enhancement New feature or request label Feb 2, 2021
@packwatcher
Copy link

Hey, you might be better off, opening a PR or asking in the discord for a better response. Also, your best bet would be to check if there is anything in the natives relating to this.

@d0p3t
Copy link
Owner

d0p3t commented Feb 2, 2021

We can definitely add this. If you plan on PRing this, make sure you write your class methods in camelCase

It should be a pretty quick job. If you aren't, I will try to find time this week.

We should probably add the respective method for an entity inside the entity's class (ped, vehicle), instead of in a Task class to keep the pattern of "this thing only belongs to a vehicle, so it's part of the vehicle object"

@lfshr
Copy link
Contributor Author

lfshr commented Feb 3, 2021

Hey, you might be better off, opening a PR or asking in the discord for a better response. Also, your best bet would be to
check if there is anything in the natives relating to this.

Hey Marco. This issue is really just here to have a bit of a design discussion before pressing buttons.

We can definitely add this. If you plan on PRing this, make sure you write your class methods in camelCase

It should be a pretty quick job. If you aren't, I will try to find time this week.

We should probably add the respective method for an entity inside the entity's class (ped, vehicle), instead of in a Task class to keep the pattern of "this thing only belongs to a vehicle, so it's part of the vehicle object"

All tasks are applied to a Ped in the natives afaik.

@d0p3t
Copy link
Owner

d0p3t commented Feb 3, 2021

I was looking at the C# wrapper and there's natives to drive a Vehicle to a certain coordinate etc. But for sequences, yes that's probably only peds. Not 100% sure on that

@lfshr
Copy link
Contributor Author

lfshr commented Feb 3, 2021

I was looking at the C# wrapper and there's natives to drive a Vehicle to a certain coordinate etc. But for sequences, yes that's probably only peds. Not 100% sure on that

I think just copy the C# stuff entirely. So we'll also get the added benefit of adding arbitrary Task abstractions to fivem-js as well.

So we could do both a sequence, like above, as well as:

myPed.Task.GoTo(target);

d0p3t added a commit that referenced this issue Feb 19, 2021
@d0p3t
Copy link
Owner

d0p3t commented Feb 19, 2021

Let me know if the above commit is the wanted structure. It should perform as explained, but I haven't tested it. PlayAnimation isn't as flexible as C#'s implementation right now (requires defining all parameters yourself).

@lfshr
Copy link
Contributor Author

lfshr commented Feb 23, 2021

Looks good! Going to go ahead and close this off.

@lfshr lfshr closed this as completed Feb 23, 2021
@d0p3t
Copy link
Owner

d0p3t commented May 21, 2021

If you @lfshr or anyone else wanting to use TaskSequence's and such could please test this in the next fivem-js version, please? Would like it to be according to your needs

@d0p3t d0p3t reopened this May 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants