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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix forever while loop (WIP) #49

Closed
wants to merge 5 commits into from

Conversation

dobrite
Copy link

@dobrite dobrite commented Aug 14, 2015

My attempt at #46 and #48. This is my best guess as to what is going on.

exec's signature is ugly so feel free to refactor.

I'd test it on my game, but I upgraded rust and am getting an error. It's late here so I'll give it a go in the morn.

cheers,
dave 馃嵒

@dobrite
Copy link
Author

dobrite commented Aug 14, 2015

Added another failing spec. Moving the return fixes it though now I'm not convinced this is the right path. Next I'm going to test with Sequences and see if they reset.

Still a WIP

@bvssvni bvssvni changed the title fix forever while loop fix forever while loop (WIP) Aug 14, 2015
@dobrite
Copy link
Author

dobrite commented Aug 15, 2015

A quick question and some thoughts:

When returning (Success, 0.0), is it meant that the action consumed all remaining time, at which point the tree waits for another event? That's the assumption I am currently working with.

At this point I'm unsure of how an instant action (returns same dt) should work when there is no remaining time. The parent node (i.e. Sequence) doesn't know how long a contained action will take (without running it) so it is forced to run it even if there is 0.0 time left. Then it doesn't matter if the action takes all time or is instant since both get 0.0 as input and output 0.0. Meaning that a Sequence of actions that all take some time will get ran all in the same event even with no remaining time.

I am unsure on how to differentiate instant actions and actions that take time when there is no remaining time.

I am thinking that instant actions should only run if there is time left but that makes the failing tests all take one more exec to pass. Obvious this is a big change. Am I on the right track, or am I missing something fundamental about how this all should work?

(still a WIP)

@bvssvni
Copy link
Member

bvssvni commented Aug 16, 2015

Instant actions should not need time to run, because a task might require arbitrary steps before stopping. This mix of instant and consumed actions is a bit confusing, so we should clarify it a bit.

Do you use the #rust_gamedev IRC chat sometimes? I'd like to discuss this so we can figure out the precise semantics.

@dobrite
Copy link
Author

dobrite commented Aug 17, 2015

I idle in #rust_gamedev as rAad. 馃榾

Just to be clear, what I am proposing is that instant actions only can run if there is time remaining, but themselves don't use any time. If dt > 0 then any number of instant actions will run, but with dt == 0.0 it'll have to wait until the next update comes along with time before running. i.e. a turn is effectively over when it hits 0.0 regardless if the action takes time or not.

I did see reference to "consumed" actions. I'll have to look closer at it and see if I can reason through it.

@bvssvni
Copy link
Member

bvssvni commented Aug 19, 2015

I wonder if we should check the remaining delta time before executing any action and return RUNNING if there is no time left.

@bvssvni
Copy link
Member

bvssvni commented Aug 19, 2015

I've written up a summary of the design #50.

I am not 100% sure yet about the semantics about instant actions, but if we change it I think it should be a check before executing the action, instead of treating it as a special case for Sequence. Opened #51.

@dobrite
Copy link
Author

dobrite commented Aug 20, 2015

Yes, I 100% agree. What I have is a special case. It would be nice for the solution to be more general.

@dobrite
Copy link
Author

dobrite commented Aug 21, 2015

Closing in favor of the options listed in #51

@dobrite dobrite closed this Aug 21, 2015
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.

None yet

2 participants