Toggle debug mode at runtime

[adrian-wozniak]

Summary

Adds setDebug and setVisualDebug methods to the SpatialNavigation to enable/disable at runtime the navigation decision logs and the canvas overlay showing bounding boxes and navigation paths.

Motivation

Very often I need to quickly check what is happening with a focus. Right now it is required to reload the app and reinitialize to spatial navigation. It is useful to debug current session and not the fresh state that is rebuilt after reload. It is also useful to disable debugger when not needed and continue working with the app without a need of reloading it.

[changeset-bot]

🦋 Changeset detected

Latest commit: ed6675e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@noriginmedia/norigin-spatial-navigation-core	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[predikament]

Thanks for the contribution @adrian-wozniak.

We'll have a look and discuss in the team if this is something we think would add value.

Appreciate the work nevertheless. 👍🏻

[adrian-wozniak]

@predikament To give you a little more context... In our TV app, in the debug mode, we have separate key code combos defined that enable/disable different debug tools. For example, pressing keys in sequence left->up->right->left->right enables/disables spatial navigation debugger.
That's very handy if we want to debug problem right away. The fact that spatial navigation debugger forces app restart means that sessions of other debug tools (like remote webkit debugger) are interrupted and need to be re-established. Long term, it's kind of annoying 😅.
When working with a regular browser, it doesn't matter much, but things get changed when we have to deal with quite old set top boxes, where focus issues are more likely to happen, native remote webkit debugger is not provided and we have to use custom "console log tracker".

[asgvard]

Hi!

I'm not sure we want to add new methods for this, even though it's a simple setter. But technically it is already possible to toggle it directly via SpatialNavigation singleton:

import { SpatialNavigation } from '@noriginmedia/norigin-spatial-navigation-core';

// Directly in the code
function setDebug(enabled = false) {
  SpatialNavigation.debug = enabled;
}

// To address it later in console
window.SpatialNavigation = SpatialNavigation;

// in console:
SpatialNavigation.debug = true;

[adrian-wozniak]

@asgvard Thanks for your answer. You are right, technically it's possible, but the debug field is marked as private, so I assumed the intention is to not modify it outside the class. Keeping that in mind, I wanted to keep API consistent as there exists setThrottle method. With setVisualDebug situation it is not that straightforward as for setDebug and setting visualDebugger via SpatialNavigation singleton seems to be not the best idea.

Would that work better for you if I would create only one method that takes two arguments and allows setting both debug methods at once? Something like setDebug(debug: boolean, visualDebug: boolean).

[adrian-wozniak]

@asgvard @predikament Do you have any updates on that, please?

@asgvard Your proposal doesn't work well in the TypeScript code base unfortunately. TypeScript reports an error about accessing private field.

[predikament]

@asgvard @predikament Do you have any updates on that, please?

@asgvard Your proposal doesn't work well in the TypeScript code base unfortunately. TypeScript reports an error about accessing private field.

Hello @adrian-wozniak,

Apologies for the slow response.

We had an internal vote on this and most of the team were not really for it, due to it bloating the package with extra code for little to no real extra value. In addition there is already a workaround for this, as mentioned by @asgvard above.

I will close this PR - Sorry it didn't work out, but we really do appreciate the contribution regardless, so please do not be hesitant to contribute in the future.

Note: If we see more votes or comments on this topic then will of course re-address it.

Thanks.