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

Implements random npc movement via a loopable queueable task #431

Merged
merged 4 commits into from
Sep 2, 2024

Conversation

charles-m-knox
Copy link
Contributor

In order to implement random NPC movements, I created a new task called QueueableTask. It is a general-purpose task that executes a callback on each tick, and also optionally allows a base task to be triggered on the next tick.

I used this to implement actor.moveSomewhere() on each tick.

Please provide any thoughts in the implementation. I have only been introduced to this code base as of yesterday, so there are likely some subtleties and nuances that I have glossed over in my implementation. There might even already be an existing task/pipe/etc that implements what I implemented here, and if so, let me know.

Copy link
Contributor Author

@charles-m-knox charles-m-knox left a comment

Choose a reason for hiding this comment

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

Left a few notes on areas I'd like to have reviewed.

callbackResult: true,
shouldContinueLooping: true,
};
}, null, { cacheOriginal: false, objectConfig: this as unknown as ObjectConfig })) // TODO: this needs to be better
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any suggestions on how I might pass in the actor (or just simply nothing) as an ObjectConfig here? I used an escape hatch in TS to get around the type checker, and I would prefer a better solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

made it possible to null this field

@@ -212,7 +212,7 @@ export class Npc extends Actor {
if(this.metadata.following) {
return false;
}
return this.updateFlags.faceActor === undefined && this.updateFlags.animation === undefined;
return this.updateFlags.faceActor === null && this.updateFlags.animation === null;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needed to be changed in order to allow NPC's to move. I evaluated their state during regular idle gameplay and these values were null. I am not familiar with the purpose of these so I may be breaking something here - please provide thoughts.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nulling instead of setting undefined here seems better, since initial is null

@charles-m-knox charles-m-knox marked this pull request as ready for review September 2, 2024 16:54
@charles-m-knox charles-m-knox changed the title Implements random npc movement via a loopable queuable task Implements random npc movement via a loopable queueable task Sep 2, 2024
@Promises Promises merged commit 179e299 into runejs:develop Sep 2, 2024
1 check passed
@charles-m-knox charles-m-knox deleted the npc-random-movements branch September 7, 2024 14:35
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