Skip to content

Conversation

JakeShirley
Copy link
Collaborator

This PR accomplishes two main things:

  • Adds support for the client memory stats (which will require a change to Minecraft, don't merge this yet!)
  • Cleans up the diagnostics prefabs to make them more readable and maintainable
    • Splits out prefabs to one tab per file to make the system more scalable

Screenshot

image

@JakeShirley JakeShirley requested review from chmeyer-ms and frgarc May 2, 2025 21:29
)}
{tabPrefab.content({ selectedClient, selectedPlugin })}
</VSCodePanelView>
))}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so cool.

// Filter out data points that are older than the latest tick
newData = newData.filter(dataPoint => {
const latestTick = latestTicks.get(dataPoint.category!);
return latestTick !== undefined && dataPoint.time === latestTick;
Copy link

@frgarc frgarc May 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit non-blocking, shouldn't this be enough?

Suggested change
return latestTick !== undefined && dataPoint.time === latestTick;
return dataPoint.time === latestTick;

Comment on lines +92 to +102
newData.sort((a, b) => {
if (selectedSortType === MinecraftStatisticTableSortType.Alphabetical) {
return selectedSortOrder === MinecraftStatisticTableSortOrder.Ascending
? a.category!.localeCompare(b.category!)
: b.category!.localeCompare(a.category!);
} else {
return selectedSortOrder === MinecraftStatisticTableSortOrder.Ascending
? a.absoluteValue - b.absoluteValue
: b.absoluteValue - a.absoluteValue;
}
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit non-blocking, for performance it would be better to split into 4 sort sentences, otherwise it is making the comparison of filter types in every nested iteration.

const statsTab: TabPrefab = {
name: 'Client - Memory',
dataSource: TabPrefabDataSource.Client,
content: ({ selectedClient }: TabPrefabParams) => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking: Would be awesome to use some react memoize mechanism to the content rendering :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants