feat: new optional contextDepth for archy renderer #92

ddurschlag6river · 2019-02-16T17:38:03Z

This adds an optional contextDepth parameter when using the archy renderer for the behavior tree.

Leaving out this optional parameter causes the existing behavior.

Describing the effect when the parameter is used is best done by example. Here are the unit tests for the new behavior, with the addition of showing the output they are testing:


  renderTree
    archy tree
      contextDepth
        before running

          ✓ should show nothing with -1 context depth
...

          ✓ should show root ellipsis with 0 context depth
root (LatchedSequence)
├─┬ 0 (LatchedSequence)
│ ├─┬ 0.0 (LatchedSequence)
│ │ ├── 0.0.0 (ConsumeOnce)
│ │ └── 0.0.1 (ConsumeOnce)
│ └─┬ 0.1 (LatchedSequence)
│   ├── 0.1.0 (ConsumeOnce)
│   └── 0.1.1 (ConsumeOnce)
└─┬ 1 (LatchedSequence)
  ├── 1.0 (ConsumeOnce)
  └── 1.1 (ConsumeOnce)

          ✓ should show everything by default
        after one run
root (LatchedSequence) => RUNNING
├─┬ 0 (LatchedSequence) => RUNNING
│ ├─┬ 0.0 (LatchedSequence) => RUNNING
│ │ ├── 0.0.0 (ConsumeOnce) => RUNNING
│ │ └── 0.0.1 (ConsumeOnce)
│ └─┬ 0.1 (LatchedSequence)
│   ├── 0.1.0 (ConsumeOnce)
│   └── 0.1.1 (ConsumeOnce)
└─┬ 1 (LatchedSequence)
  ├── 1.0 (ConsumeOnce)
  └── 1.1 (ConsumeOnce)

          ✓ should arrow the first path by default
root (LatchedSequence) => RUNNING
├─┬ 0 (LatchedSequence) => RUNNING
│ ├─┬ 0.0 (LatchedSequence) => RUNNING
│ │ ├── 0.0.0 (ConsumeOnce) => RUNNING
│ │ └── ...
│ └── ...
└── ...

          ✓ should show only the active path and ellipses 0 context depth
root (LatchedSequence) => RUNNING
└─┬ 0 (LatchedSequence) => RUNNING
  └─┬ 0.0 (LatchedSequence) => RUNNING
    └── 0.0.0 (ConsumeOnce) => RUNNING

          ✓ should show only the active path at -1 context depth
root (LatchedSequence) => RUNNING
├─┬ 0 (LatchedSequence) => RUNNING
│ ├─┬ 0.0 (LatchedSequence) => RUNNING
│ │ ├── 0.0.0 (ConsumeOnce) => RUNNING
│ │ └── 0.0.1 (ConsumeOnce)
│ └─┬ 0.1 (LatchedSequence)
│   ├── ...
│   └── ...
└─┬ 1 (LatchedSequence)
  ├── ...
  └── ...

          ✓ should show only the active path, siblings, and ellipses
root (LatchedSequence) => RUNNING
├─┬ 0 (LatchedSequence) => RUNNING
│ ├─┬ 0.0 (LatchedSequence) => RUNNING
│ │ ├── 0.0.0 (ConsumeOnce) => RUNNING
│ │ └── 0.0.1 (ConsumeOnce)
│ └─┬ 0.1 (LatchedSequence)
│   ├── 0.1.0 (ConsumeOnce)
│   └── 0.1.1 (ConsumeOnce)
└─┬ 1 (LatchedSequence)
  ├── 1.0 (ConsumeOnce)
  └── 1.1 (ConsumeOnce)

          ✓ should show everything at 2 context depth


  8 passing (25ms)

codecov · 2019-02-17T17:09:11Z

Codecov Report

Merging #92 into develop will increase coverage by 0.17%.
The diff coverage is 96.96%.

@@             Coverage Diff             @@
##           develop      #92      +/-   ##
===========================================
+ Coverage    89.85%   90.03%   +0.17%     
===========================================
  Files           18       18              
  Lines          276      291      +15     
  Branches        38       47       +9     
===========================================
+ Hits           248      262      +14     
- Misses          23       24       +1     
  Partials         5        5

Impacted Files	Coverage Δ
lib/utils/renderTree.ts	`100% <100%> (ø)`	⬆️
lib/utils/archyTree.ts	`96.77% <96.29%> (-3.23%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3100ca4...b6b162f. Read the comment docs.

jbcpollak

comments, nothing major

jbcpollak · 2019-02-17T17:01:46Z

lib/utils/archyTree.ts

@@ -4,40 +4,64 @@ import {BlueshellState} from '../nodes/BlueshellState';
 import * as archy from 'archy';
 import {Data} from 'archy';

-function buildArchyTree<S extends BlueshellState, E>(node: Base<S, E>, state?: S): Data {
+function buildArchyTree<S extends BlueshellState, E>(
+	node: Base<S, E>, state?: S, contextDepth = Number.MAX_SAFE_INTEGER


does this function need a default contextDepth? Its not exported and is only accessed by serializeArchyTree()

jbcpollak · 2019-02-17T17:02:20Z

lib/utils/archyTree.ts

-		for (let child of (<any>node).children) {
-			archyTree.nodes.push(buildArchyTree(<Base<S,E>>child, state));
+		for (const child of (<any>node).children) {
+			const subTree = buildArchyTree(<Base<S, E>>child, state, contextDepth - (onPath ? 0 : 1));


I would split the contextDepth calculation in't its own const declaration for clarity and to add a comment to describe the logic.

jbcpollak · 2019-02-17T17:02:35Z

lib/utils/archyTree.ts

+	tree: Base<S, E>, state?: S, contextDepth = Number.MAX_SAFE_INTEGER
+): string {
+	const archyTree = buildArchyTree(tree, state, contextDepth);
+	if ( archyTree ) {


shouldn't have spaces here, did the linter not catch that?

Guess not. I rely heavily on my VSCode linter plugin, which seems to work reliably with almost all our repos, but occasionally gets pissy and doesn't like one or another. Given that this one is configured differently, I wouldn't be shocked if it wasn't auto-cleaning-up after me (though I thought I saw it doing that...). Could also just be that the lint config here is different.

lib/utils/renderTree.ts

feat: new optional contextDepth for archy renderer

002ccb5

ddurschlag6river requested review from jbcpollak, Shockolate and a team February 16, 2019 17:38

ddurschlag6river added 3 commits February 16, 2019 12:39

fix: remove leaked debugging code

77095fb

fix: re-order and rename tests

e6c0647

fix: update TS version for Required<T> and other goodies

165cbc3

jbcpollak reviewed Feb 17, 2019

View reviewed changes

fix: code review feedback

b6b162f

jbcpollak approved these changes Feb 17, 2019

View reviewed changes

ddurschlag6river merged commit 56e91ce into develop Feb 18, 2019

ddurschlag6river deleted the archyContextDepth branch February 18, 2019 02:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: new optional contextDepth for archy renderer #92

feat: new optional contextDepth for archy renderer #92

ddurschlag6river commented Feb 16, 2019 •

edited

Loading

codecov bot commented Feb 17, 2019 •

edited

Loading

jbcpollak left a comment

jbcpollak Feb 17, 2019

jbcpollak Feb 17, 2019

ddurschlag6river Feb 17, 2019

jbcpollak Feb 17, 2019

ddurschlag6river Feb 17, 2019

feat: new optional contextDepth for archy renderer #92

feat: new optional contextDepth for archy renderer #92

Conversation

ddurschlag6river commented Feb 16, 2019 • edited Loading

codecov bot commented Feb 17, 2019 • edited Loading

Codecov Report

jbcpollak left a comment

Choose a reason for hiding this comment

jbcpollak Feb 17, 2019

Choose a reason for hiding this comment

jbcpollak Feb 17, 2019

Choose a reason for hiding this comment

ddurschlag6river Feb 17, 2019

Choose a reason for hiding this comment

jbcpollak Feb 17, 2019

Choose a reason for hiding this comment

ddurschlag6river Feb 17, 2019

Choose a reason for hiding this comment

ddurschlag6river commented Feb 16, 2019 •

edited

Loading

codecov bot commented Feb 17, 2019 •

edited

Loading