feat(diagnostics): Ability to load a replay of diagnostics events from file #270
Conversation
src/stats/stats-provider.ts
Outdated
| public start() {} | ||
| public stop() {} | ||
| public pause() {} | ||
| public resume() {} | ||
| public faster() {} | ||
| public slower() {} |
There was a problem hiding this comment.
nit: make these throw in the base so it's obvious when the wrong listener is being used? Mostly useful for future refactoring or additions
src/stats/stats-provider.ts
Outdated
| public getName(): string { | ||
| return this._name; | ||
| } | ||
|
|
||
| public getUniqueId(): string { | ||
| return this._uniqueId; | ||
| } |
There was a problem hiding this comment.
These fields are fully static and can never change right? You could make them public readonly then to be more concise
class StatsListener {
constructor (public readonly name: string,
public readonly uniqueId: string) {
this._statListeners = [];
};
}
const stts = new StatsListener('test', 'test', []);
stts.name = 'blah'; // disallowed, will be an error
| }; | ||
| } | ||
|
|
||
| const ReplayControls: React.FC<ReplayControlsProps> = ({ |
There was a problem hiding this comment.
Full disclosure, I have no idea what that means :). But I'd be happy to do it.
There was a problem hiding this comment.
Just wrap the component you create in a call to memo
Basically caches it for each rendering of it, especially if properties don't change much.
There was a problem hiding this comment.
IC... I guess I should do that to all the components then.
There was a problem hiding this comment.
It can usually be done to a lot of components, but has a cost associated with it (caching a whole component and props comparison), but it can be a perf win. It's usually only undesirable if the properties change super often on re-render.
| private readonly MIN_SPEED = 5; | ||
| private readonly MAX_SPEED = 160; |
There was a problem hiding this comment.
should these be customizable for folks? Same with default speed?
There was a problem hiding this comment.
I considered that, but slower than 5 was barely noticeable. And faster than 160 was unusable. I think fine for now.
src/panels/minecraft-diagnostics.ts
Outdated
| default: | ||
| break; |
src/stats/replay-stats-provider.ts
Outdated
| } else if (this._simTickFreqency > this.MAX_SPEED) { | ||
| this._simTickFreqency = this.MAX_SPEED; | ||
| } | ||
| this._simTickPeriod = this.MILLIS_PER_SECOND / this._simTickFreqency; |
There was a problem hiding this comment.
This is a candidate to have its own method and avoid same code in other places, even it is one line.
|
|
||
| const handlePluginSelection = useCallback((pluginSelectionId: string) => { | ||
| console.log(`Selected Plugin: ${pluginSelectionId}`); |
There was a problem hiding this comment.
Not exactly, just leftover debug message from my last PR
Looks like this...