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

Permit Variable Movement and Rotation Amounts, Simulating Physics and Running Across Multiple Frames Appropriately #235

Open
ThomasSchellenbergNextCentury opened this issue Feb 8, 2021 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@ThomasSchellenbergNextCentury
Copy link
Collaborator

ThomasSchellenbergNextCentury commented Feb 8, 2021

From: Amir, Alan, others?

Priority: Eval 4

Components: Python API, Unity Env

Blockers:

Background: Right now, Move actions only move the performer 0.1 unit, and Rotate/Look actions only rotate the performer 10 degrees.

Goal: Let performers provide parameters to Move, Rotate, and Look actions that allow them to move/rotate across multiple time steps with a single action. For example, a MoveAhead action with an amount of 1 would move the performer forward 1 unit over 10 time steps (0.1 each) and THEN return the step output. It's essentially like calling the same action multiple times in a row. This only needs to be done for the Move, Rotate, and Look actions.

Technical Suggestions:

  • In MCSController.SimulatePhysicsOnce, if the move or rotate amount is above the max, only move or rotate the max. (The rotation happens in RotateLookAcrossFrames and RotateLookBodyAcrossFrames.) Make note if you didn't move/rotate the full amount from the action's input (maybe reusing the movementActionData, lookRotationActionData, and bodyRotationActionData variables?).
  • In MCSController.SimulatePhysics, rather than always calling FinalizeEmit, if there's still any movement/rotation that must be done, do it BEFORE calling FinalizeEmit. I think you could just call SimulatePhysics again, recursively? Since all of the movement and rotation is already done in SimulatePhysicsOnce, you shouldn't (?) need to call ProcessControlCommand again, but you WILL need to increment this.step for each recursive call.
  • Ignore anything in MCSController concerning the "substep". We only have one substep now, and that doesn't need to change. (I should really make a separate ticket to remove all of the "substep" code.)
  • Originally this ticket said to avoid taking screenshots until the action is completely finished. However, I think it would be useful to include the screenshots after each movement/rotation iteration, at least in the initial implementation. This should mean making fewer changes to the code, too! I think we'll need feedback from TA1 on how many images they want returned. We can always strip out the extra intermediary images later.

Other Notes: Moves of less than 0.1 should still cost them 1 time step, along with Rotates/Looks of less than +/- 10 degrees, so the performers can’t have any “free” actions.
Al use-case would be to allow the user to fully move across the room (provided no obstructions)

Slack links:

TA2 internal JIRA tickets:

@deanwetherby deanwetherby added the enhancement New feature or request label Mar 9, 2021
@deanwetherby deanwetherby added this to the Eval 4 milestone Mar 9, 2021
@erick-u
Copy link
Contributor

erick-u commented Mar 25, 2021

@deanwetherby @ThomasSchellenbergNextCentury Do we wish to still support set increments of movement and rotation? Example being movement always rounds to 0.1 and rotation rounds to 10th degree.

Movement:
0 = 0.1
1.01 = 1.1

Rotation:
0 = 10
11 = 20

The reason I ask is that as this task is supported, I am unsure if there is a specific reason to have these constraints. The idea of it taking a single "action" regardless of the amount of movement in terms of scoring means there may be no need to set any real limits.

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

Successfully merging a pull request may close this issue.

3 participants