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

Roadmap for replace Pathfinding with FlexiblePathfinding #4981

Closed
14 tasks done
DarkWeird opened this issue Jan 13, 2022 · 7 comments
Closed
14 tasks done

Roadmap for replace Pathfinding with FlexiblePathfinding #4981

DarkWeird opened this issue Jan 13, 2022 · 7 comments
Labels
Category: Doc Requests, Issues and Changes targeting javadoc and module documentation Status: Needs Discussion Requires help discussing a reported issue or provided PR

Comments

@DarkWeird
Copy link
Contributor

DarkWeird commented Jan 13, 2022

Motivation

  • High memory usage of Pathfinding(especially in java)
  • Unnecessary PF's NavGraph usage (e.g. CoreGameplay don't needs it, but NavGraph is creates)
  • We have alternative already (FPF)

Concerns

  • Is there specific expertise that will be needed for this effort?
  • Does this effort have dependencies on other efforts?
  • Do you expect this effort to conflict with any other efforts?
  • What are potential drawbacks of the effort?
  • What are maintenance or continuous efforts that will persist beyond the completion of this effort?

Task Breakdown

Optional tasks and improvements

  • Move code and features from Gooyes Defence to FlexableMoving.
  • Provide documentation for FPF.
@DarkWeird DarkWeird added Category: Doc Requests, Issues and Changes targeting javadoc and module documentation Status: Needs Discussion Requires help discussing a reported issue or provided PR labels Jan 13, 2022
@skaldarnar
Copy link
Member

We went for a more ... straight ahead path now by integrating what was already done for https://github.com/Terasology/FlexibleMovement back into https://github.com/Terasology/Behaviors.

GooeyDefence already uses FlexiblePathfinding directly, so I guess that step is not required, but might be a good test for the new behaviors.

@skaldarnar
Copy link
Member

Update: Current state of the migration (with a couple of local changes I'll need to push later when back home) I can start the game again, although there are still failing tests and some worrying log messages and non-optimal behavior of mobs, e.g., with WildAnimals.

  • 4 failing Behaviors tests

     MovementTests. [walking, leaping]: leap
     MovementTests. [swimming]: leap
     MovementTests. [swimming]: diagonal
     MovementTests. [swimming]: straight
    
  • injection @In not working for behavior actions

     18:40:23.697 [main] WARN  o.t.engine.registry.InjectionHelper - SetTargetToNearbyBlockNode wanted PluginSystem injected but CoreRegistry has none.
     18:40:23.697 [main] WARN  o.t.engine.registry.InjectionHelper - MoveToAction wanted WorldProvider injected but CoreRegistry has none.
     ...
     18:40:23.769 [main] WARN  o.t.engine.registry.InjectionHelper - FindPathToNode wanted PathfinderSystem injected but CoreRegistry has none.
    
  • broken prefabs?

     18:40:23.698 [main] WARN  o.t.e.p.serializers.PrefabSerializer - Prefab 'WildAnimals:babyDeer' contains unknown component 'StrayIfIdle'
     18:40:23.505 [main] WARN  o.t.e.p.serializers.PrefabSerializer - Prefab 'Behaviors:jobBuildBlock' contains unknown component 'Work
    
  • broken behavior trees or behavior actions? (IllegalArgumentException is silently swallowed... o.O)

     18:52:40.472 [main] WARN  o.t.e.physics.bullet.BulletPhysics - Can't update Trigger entity with a non-finite position/rotation?! Entity: EntityRef{id = 3169, netId = 0, prefab = 'WildAnimals:sheep'}
    
     18:11:07.457 [Test worker] DEBUG org.terasology.engine.core.ComponentSystemManager - Initialising org.terasology.engine.logic.behavior.BehaviorSystem@2ffebb03
     18:11:07.460 [Test worker] ERROR org.terasology.engine.core.ComponentSystemManager - Failed to initialise system org.terasology.engine.logic.behavior.BehaviorSystem@2ffebb03
     java.lang.IllegalArgumentException: Unknown behavior node type setup_continuous_pathfinding
    
     18:11:07.466 [Test worker] DEBUG org.terasology.engine.core.ComponentSystemManager - Initialising org.terasology.engine.logic.behavior.CollectiveBehaviorSystem@c892b6
     18:11:07.466 [Test worker] ERROR org.terasology.engine.core.ComponentSystemManager - Failed to initialise system org.terasology.engine.logic.behavior.CollectiveBehaviorSystem@c892b6
     java.lang.IllegalArgumentException: Unknown behavior node type setup_continuous_pathfinding
    

@jdrueckert
Copy link
Member

jdrueckert commented Jan 17, 2022

* broken prefabs?
  ```
   18:40:23.698 [main] WARN  o.t.e.p.serializers.PrefabSerializer - Prefab 'WildAnimals:babyDeer' contains unknown component 'StrayIfIdle'
   18:40:23.505 [main] WARN  o.t.e.p.serializers.PrefabSerializer - Prefab 'Behaviors:jobBuildBlock' contains unknown component 'Work
  ```

@skaldarnar Terasology/WildAnimals#93 addresses the first of the two.

The second originates from the WorkComponent having been removed without replacement in Terasology/Behaviors#88. I don't see any usages of jobBuildBlock.prefab or jobWalkToBlock.prefab, which are the only references to this component in omega. I would propose to remove the prefabs.

@jdrueckert
Copy link
Member

jdrueckert commented Jan 17, 2022

* broken behavior trees or behavior actions? (IllegalArgumentException is silently swallowed... o.O)
  ```
   18:52:40.472 [main] WARN  o.t.e.physics.bullet.BulletPhysics - Can't update Trigger entity with a non-finite position/rotation?! Entity: EntityRef{id = 3169, netId = 0, prefab = 'WildAnimals:sheep'}
  
   18:11:07.457 [Test worker] DEBUG org.terasology.engine.core.ComponentSystemManager - Initialising org.terasology.engine.logic.behavior.BehaviorSystem@2ffebb03
   18:11:07.460 [Test worker] ERROR org.terasology.engine.core.ComponentSystemManager - Failed to initialise system org.terasology.engine.logic.behavior.BehaviorSystem@2ffebb03
   java.lang.IllegalArgumentException: Unknown behavior node type setup_continuous_pathfinding
  
   18:11:07.466 [Test worker] DEBUG org.terasology.engine.core.ComponentSystemManager - Initialising org.terasology.engine.logic.behavior.CollectiveBehaviorSystem@c892b6
   18:11:07.466 [Test worker] ERROR org.terasology.engine.core.ComponentSystemManager - Failed to initialise system org.terasology.engine.logic.behavior.CollectiveBehaviorSystem@c892b6
   java.lang.IllegalArgumentException: Unknown behavior node type setup_continuous_pathfinding
  ```

This is due to setup_continuous_pathfinding being removed in Terasology/Behaviors#89 (https://github.com/Terasology/Behaviors/blob/d3600d101140c2f012aeb9ebc1d2c210f67cff02/src/main/java/org/terasology/module/behaviors/actions/SetupContinuousMoveNode.java).

@skaldarnar skaldarnar changed the title Roadmap for replace Pathfinding with FlexableMovement Roadmap for replace Pathfinding with FlexibleMovement Jan 21, 2022
@skaldarnar
Copy link
Member

I think with merging Terasology/Behaviors#89 and related PRs we're done with the main list of tasks as described in this issue. I'll copy over the respective follow-ups from that issue description such that we can keep track of it here (will update the initial issue).

@jdrueckert jdrueckert changed the title Roadmap for replace Pathfinding with FlexibleMovement Roadmap for replace Pathfinding with FlexiblePathfinding Jan 21, 2022
jdrueckert added a commit to Terasology/Index that referenced this issue Jan 21, 2022
part of MovingBlocks/Terasology#4981 which is replacing Pathfinding with FlexiblePathfinding
jdrueckert added a commit to Terasology/Index that referenced this issue Jan 21, 2022
part of MovingBlocks/Terasology#4981 which is replacing Pathfinding with FlexiblePathfinding
skaldarnar added a commit to Terasology/Behaviors that referenced this issue Mar 13, 2022
- style: format `.behavior` files
- chore: add TODO ideas and questions in several places
- chore: add some debug logging
- feature/fix: small adjustments to Behaviors logic

Follow-up PR to #89.

Related PRs (extracted from this): #94, #96, #97, #99.

Contributes to MovingBlocks/Terasology#4981.

Depends on Terasology/FlexiblePathfinding#26 and Terasology/FlexiblePathfinding#27.
@jdrueckert
Copy link
Member

Regarding the lacking support for dependency injection in behavior actions, I noticed, that we do have something in that direction in BehaviorTreeBuilder::addAction.
However, I don't know whether - and if not why - this is sufficient.

I wonder whether this might be related to the respective fields not being registered, yet, rather than the dependency injection not working as expected...?

If we are sure that the fields are registered and the dependency injection is implemented incorrectly, #5003 might be related.

@jdrueckert
Copy link
Member

All documented follow-ups done, closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Doc Requests, Issues and Changes targeting javadoc and module documentation Status: Needs Discussion Requires help discussing a reported issue or provided PR
Projects
None yet
Development

No branches or pull requests

3 participants