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

Task manager and task resource #217

Conversation

TheSecondReal0
Copy link
Member

@TheSecondReal0 TheSecondReal0 commented Nov 24, 2020

The task system has basic functionality and has groundwork laid for future development. The InteractTask resource is used the same way as other interact resources, see #209. The biggest change coming in this PR is that init_resource(self) should be called under _ready() of its attached node. This allows the task resource to notify the task manager of its existence when the map is loaded.

It is up to reviewers if this should be merged right now or if it should wait until it is more fully featured.

HOW IT WORKS

The resource will take what's been set in the editor and translate it into information to give to the task manager. The task manager will then process this information and register a new task under a unique task ID.

Currently, nothing is done with tasks but groundwork is laid to advance task states. This doesn't happen yet because it requires task UI to integrate with the task system as well, which is beyond the scope of this PR.

WHAT'S LEFT

  • Task assignment
  • Task prerequisites (items, conditions, etc.)
  • Task outputs (items, map interactions, other tasks, etc.)
  • Two way integration. Right now only the resource integrates with the task manager, in the future task UIs must do this as well. I plan to set it up so that task UIs integrate with the task resource, which then integrates with the task manager.

NOTE: The game crashes when you attempt to transition the game to the test map, this is because of #214 and NOT this PR. See #220 for the fix.

@TheSecondReal0 TheSecondReal0 marked this pull request as draft November 24, 2020 21:53
@github-actions
Copy link

HOW TO REVIEW:

  • Test multiplayer (client and host)
  • Check build status
  • Check UI
  • Check the code
  • Check build artifacts

Because it is more flexible and easily extended without concrete task types
* Now uses InteractUI for the task UI
* Output tasks are now references to nodes which should have tasks attached
* Generally cleaned up code in standbutton and interactpoint
* Cleaned up a little in interacttask
* Added methods to interacttask for consistency
* Added init_resource to all resources. This should be called under ready of the nodes they are attached to. Self should be passed as an input to this function.
* init_resource will be used to register tasks with the taskmanager, there is very little use for it for other resources, except resources that need access to the scene tree like InteractMap and tasks
This was linked to issues Nov 27, 2020
TheSecondReal0 and others added 4 commits November 28, 2020 02:29
@TheSecondReal0 TheSecondReal0 marked this pull request as ready for review November 28, 2020 07:56
Copy link
Contributor

@nicemicro nicemicro left a comment

Choose a reason for hiding this comment

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

I have tested it with rebasing it on top of PR-220 (which I have just approved for merge), and while I can't comment on code quality due to my lack of proper understanding of the resource system, I have checked and things work.
One question I have is:
what is the expected behavior when we are on top of the clockset task that doesn't use the task system?

@TheSecondReal0
Copy link
Member Author

One question I have is:
what is the expected behavior when we are on top of the clockset task that doesn't use the task system?

It should behave exactly as it did before, I haven't changed anything about that. I haven't switched it yet because the task system doesn't support the behavior of the clockset task. Soon I'll add support for the task UI to send data through the task resource into a map interaction upon completion, which is when the standbutton clockset task can be migrated.

@nicemicro
Copy link
Contributor

It should behave exactly as it did before,

All right, so sometimes it gets randomly assigned to someone and sometimes it doesn't, and in the first case it works as it used to, on the latter, there's no interaction.
So everything works as intended.

nicemicro
nicemicro previously approved these changes Nov 29, 2020
TheSecondReal0 added a commit that referenced this pull request Nov 30, 2020
@TheSecondReal0
Copy link
Member Author

All meaningful commits included in #222, closing

@nicemicro nicemicro removed a link to an issue Feb 17, 2021
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.

Task management system
2 participants