-
Notifications
You must be signed in to change notification settings - Fork 722
Sab #2519
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
base: main
Are you sure you want to change the base?
Sab #2519
Changes from all commits
5e264a5
a31a6a0
e3a6e0b
7e6717c
f710c78
bd30070
a04abbe
bd6dbd3
0ffe9c3
2dde262
391f19d
2f2a12e
05181d7
314d8ef
37315e5
0da975f
19e5046
b312aa3
09afa11
e387820
99b01d8
430d856
fad87a9
9892537
2d63dcf
8bc140e
a4094de
fe927b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -129,6 +129,26 @@ export class TickMetricsEvent implements GameEvent { | |
| constructor( | ||
| public readonly tickExecutionDuration?: number, | ||
| public readonly tickDelay?: number, | ||
| // Number of turns the client is behind the server (if known) | ||
| public readonly backlogTurns?: number, | ||
| // Number of simulation ticks applied since last render | ||
| public readonly ticksPerRender?: number, | ||
| // Approximate worker simulation ticks per second | ||
| public readonly workerTicksPerSecond?: number, | ||
| // Approximate render tick() calls per second | ||
| public readonly renderTicksPerSecond?: number, | ||
| // Tile update metrics | ||
| public readonly tileUpdatesCount?: number, | ||
| public readonly ringBufferUtilization?: number, | ||
| public readonly ringBufferOverflows?: number, | ||
| public readonly ringDrainTime?: number, | ||
| ) {} | ||
| } | ||
|
|
||
| export class BacklogStatusEvent implements GameEvent { | ||
| constructor( | ||
| public readonly backlogTurns: number, | ||
| public readonly backlogGrowing: boolean, | ||
| ) {} | ||
|
Comment on lines
+148
to
152
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this belongs in InputHandler either |
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -229,7 +229,18 @@ export class PerformanceOverlay extends LitElement implements Layer { | |
| this.setVisible(this.userSettings.performanceOverlay()); | ||
| }); | ||
| this.eventBus.on(TickMetricsEvent, (event: TickMetricsEvent) => { | ||
| this.updateTickMetrics(event.tickExecutionDuration, event.tickDelay); | ||
| this.updateTickMetrics( | ||
| event.tickExecutionDuration, | ||
| event.tickDelay, | ||
| event.backlogTurns, | ||
| event.ticksPerRender, | ||
| event.workerTicksPerSecond, | ||
| event.renderTicksPerSecond, | ||
| event.tileUpdatesCount, | ||
| event.ringBufferUtilization, | ||
| event.ringBufferOverflows, | ||
| event.ringDrainTime, | ||
| ); | ||
| }); | ||
| } | ||
|
|
||
|
|
@@ -312,6 +323,14 @@ export class PerformanceOverlay extends LitElement implements Layer { | |
| this.layerStats.clear(); | ||
| this.layerBreakdown = []; | ||
|
|
||
| // reset tile metrics | ||
| this.tileUpdatesPerRender = 0; | ||
| this.tileUpdatesPeak = 0; | ||
| this.ringBufferUtilization = 0; | ||
| this.ringBufferOverflows = 0; | ||
| this.ringDrainTime = 0; | ||
| this.totalTilesUpdated = 0; | ||
|
|
||
| this.requestUpdate(); | ||
| }; | ||
|
|
||
|
|
@@ -418,7 +437,48 @@ export class PerformanceOverlay extends LitElement implements Layer { | |
| this.layerBreakdown = breakdown; | ||
| } | ||
|
|
||
| updateTickMetrics(tickExecutionDuration?: number, tickDelay?: number) { | ||
| @state() | ||
| private backlogTurns: number = 0; | ||
|
|
||
| @state() | ||
| private ticksPerRender: number = 0; | ||
|
|
||
| @state() | ||
| private workerTicksPerSecond: number = 0; | ||
|
|
||
| @state() | ||
| private renderTicksPerSecond: number = 0; | ||
|
|
||
| @state() | ||
| private tileUpdatesPerRender: number = 0; | ||
|
|
||
| @state() | ||
| private tileUpdatesPeak: number = 0; | ||
|
|
||
| @state() | ||
| private ringBufferUtilization: number = 0; | ||
|
|
||
| @state() | ||
| private ringBufferOverflows: number = 0; | ||
|
|
||
| @state() | ||
| private ringDrainTime: number = 0; | ||
|
|
||
| @state() | ||
| private totalTilesUpdated: number = 0; | ||
|
|
||
| updateTickMetrics( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should be an interface, it's too easy to mix up arguments with this many parameters.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. most of these can/should be removed after evaluation |
||
| tickExecutionDuration?: number, | ||
| tickDelay?: number, | ||
| backlogTurns?: number, | ||
| ticksPerRender?: number, | ||
| workerTicksPerSecond?: number, | ||
| renderTicksPerSecond?: number, | ||
| tileUpdatesCount?: number, | ||
| ringBufferUtilization?: number, | ||
| ringBufferOverflows?: number, | ||
| ringDrainTime?: number, | ||
| ) { | ||
| if (!this.isVisible || !this.userSettings.performanceOverlay()) return; | ||
|
|
||
| // Update tick execution duration stats | ||
|
|
@@ -455,6 +515,42 @@ export class PerformanceOverlay extends LitElement implements Layer { | |
| } | ||
| } | ||
|
|
||
| if (backlogTurns !== undefined) { | ||
| this.backlogTurns = backlogTurns; | ||
| } | ||
|
|
||
| if (ticksPerRender !== undefined) { | ||
| this.ticksPerRender = ticksPerRender; | ||
| } | ||
|
|
||
| if (workerTicksPerSecond !== undefined) { | ||
| this.workerTicksPerSecond = workerTicksPerSecond; | ||
| } | ||
|
|
||
| if (renderTicksPerSecond !== undefined) { | ||
| this.renderTicksPerSecond = renderTicksPerSecond; | ||
| } | ||
|
|
||
| if (tileUpdatesCount !== undefined) { | ||
| this.tileUpdatesPerRender = tileUpdatesCount; | ||
| this.tileUpdatesPeak = Math.max(this.tileUpdatesPeak, tileUpdatesCount); | ||
| this.totalTilesUpdated += tileUpdatesCount; | ||
| } | ||
|
|
||
| if (ringBufferUtilization !== undefined) { | ||
| this.ringBufferUtilization = | ||
| Math.round(ringBufferUtilization * 100) / 100; | ||
| } | ||
|
|
||
| if (ringBufferOverflows !== undefined && ringBufferOverflows !== 0) { | ||
| // Remember that an overflow has occurred at least once this run. | ||
| this.ringBufferOverflows = 1; | ||
| } | ||
scamiv marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (ringDrainTime !== undefined) { | ||
| this.ringDrainTime = Math.round(ringDrainTime * 100) / 100; | ||
| } | ||
|
|
||
| this.requestUpdate(); | ||
| } | ||
|
|
||
|
|
@@ -485,6 +581,14 @@ export class PerformanceOverlay extends LitElement implements Layer { | |
| executionSamples: [...this.tickExecutionTimes], | ||
| delaySamples: [...this.tickDelayTimes], | ||
| }, | ||
| tiles: { | ||
| updatesPerRender: this.tileUpdatesPerRender, | ||
| peakUpdates: this.tileUpdatesPeak, | ||
| ringBufferUtilization: this.ringBufferUtilization, | ||
| ringBufferOverflows: this.ringBufferOverflows, | ||
| ringDrainTimeMs: this.ringDrainTime, | ||
| totalTilesUpdated: this.totalTilesUpdated, | ||
| }, | ||
| layers: this.layerBreakdown.map((layer) => ({ ...layer })), | ||
| }; | ||
| } | ||
|
|
@@ -600,6 +704,37 @@ export class PerformanceOverlay extends LitElement implements Layer { | |
| <span>${this.tickDelayAvg.toFixed(2)}ms</span> | ||
| (max: <span>${this.tickDelayMax}ms</span>) | ||
| </div> | ||
| <div class="performance-line"> | ||
| Worker ticks/s: | ||
| <span>${this.workerTicksPerSecond.toFixed(1)}</span> | ||
| </div> | ||
| <div class="performance-line"> | ||
| Render ticks/s: | ||
| <span>${this.renderTicksPerSecond.toFixed(1)}</span> | ||
| </div> | ||
| <div class="performance-line"> | ||
| Ticks per render: | ||
| <span>${this.ticksPerRender}</span> | ||
| </div> | ||
| <div class="performance-line"> | ||
| Backlog turns: | ||
| <span>${this.backlogTurns}</span> | ||
| </div> | ||
| <div class="performance-line"> | ||
| Tile updates/render: | ||
| <span>${this.tileUpdatesPerRender}</span> | ||
| (peak: <span>${this.tileUpdatesPeak}</span>) | ||
| </div> | ||
| <div class="performance-line"> | ||
| Ring buffer: | ||
| <span>${this.ringBufferUtilization}%</span> | ||
| (${this.totalTilesUpdated} total, ${this.ringBufferOverflows} | ||
| overflows) | ||
| </div> | ||
| <div class="performance-line"> | ||
| Ring drain time: | ||
| <span>${this.ringDrainTime.toFixed(2)}ms</span> | ||
| </div> | ||
|
Comment on lines
+707
to
+737
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major Use The new performance lines (707-737) use hardcoded English strings like "Worker ticks/s:", "Render ticks/s:", etc. For consistency with the rest of the overlay, use <div class="performance-line">
${translateText("performance_overlay.worker_ticks")}
<span>${this.workerTicksPerSecond.toFixed(1)}</span>
</div>Add corresponding keys to "performance_overlay": {
"worker_ticks": "Worker ticks/s:",
"render_ticks": "Render ticks/s:",
"ticks_per_render": "Ticks per render:",
"backlog_turns": "Backlog turns:",
"tile_updates": "Tile updates/render:",
"ring_buffer": "Ring buffer:",
"ring_drain_time": "Ring drain time:"
}🤖 Prompt for AI Agents |
||
| ${this.layerBreakdown.length | ||
| ? html`<div class="layers-section"> | ||
| <div class="performance-line"> | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this data relevant to InputHandler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Semantically, no, but it does extend TickMetricsEvent which was here before.
Most of this is only for the perf overlay and can be removed after testing.