Fix branchedResetPropagation #57

Merged
thepaperpilot merged 3 commits from nif/Profectus-Niffix:main into main 2024-02-14 17:39:07 +00:00

View file

@ -338,34 +338,21 @@ export const branchedResetPropagation = function (
tree: GenericTree,
resettingNode: GenericTreeNode
): void {
const visitedNodes = [resettingNode];
let currentNodes = [resettingNode];
if (tree.branches != null) {
const branches = unref(tree.branches);
while (currentNodes.length > 0) {
const nextNodes: GenericTreeNode[] = [];
currentNodes.forEach(node => {
branches
.filter(branch => branch.startNode === node || branch.endNode === node)
.map(branch => {
if (branch.startNode === node) {
return branch.endNode;
}
return branch.startNode;
})
.filter(node => !visitedNodes.includes(node))
.forEach(node => {
// Check here instead of in the filter because this check's results may
// change as we go through each node
if (!nextNodes.includes(node)) {
nextNodes.push(node);
node.reset?.reset();
}
});
});
currentNodes = nextNodes;
visitedNodes.push(...currentNodes);
const links = unref(tree.branches);
if (links == null) return;

instead of === undefined, can you use == null? That matches the rest of the codebase and has subtly different behavior.

instead of `=== undefined`, can you use `== null`? That matches the rest of the codebase and has subtly different behavior.
Outdated
Review

You very well could, I didn't realise that == null tested for undefined

You very well could, I didn't realise that `== null` tested for `undefined`
const reset: GenericTreeNode[] = [];
let current = [resettingNode];
while (current.length != 0) {
const next: GenericTreeNode[] = [];

The linter says next, node, and link should all be const

The linter says next, node, and link should all be `const`
Outdated
Review

that's probably because i use Array.prototype.concat and Array.prototype.push instead of reassigning them

that's probably because i use `Array.prototype.concat` and `Array.prototype.push` instead of reassigning them
for (const node of current) {
for (const link of links.filter(link => link.startNode === node)) {
if ([...reset, ...current].includes(link.endNode)) continue
next.push(link.endNode);
link.endNode.reset?.reset();
}
};
reset.push(...current);

This line is fine, but I wonder why you don't just push like you do the next array.

This line is fine, but I wonder why you don't just `push` like you do the `next` array.
Outdated
Review

because then that gives a shaped array that can't be iterated over as well

because then that gives a shaped array that can't be iterated over as well
Outdated
Review

wait no i could have .push(...'ed

wait no i could have `.push(...`'ed
current = next;
}
};