Skip to content
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

feat: Add TimeTrackComponent and ChildCounterComponent #2846

Merged
merged 13 commits into from Nov 21, 2023

Conversation

erickzanardo
Copy link
Member

@erickzanardo erickzanardo commented Nov 10, 2023

Description

Adds TimeTrackComponent and a ChildCounterComponent to help developers track down perfomance issues in their game.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Related Issues

Replace or remove this text.

@erickzanardo erickzanardo marked this pull request as draft November 10, 2023 12:39
@spydon spydon changed the title feat: time track component feat: Time track component Nov 10, 2023
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Looks good, no harm in providing debug helpers!
Maybe they could reside in a subdirectory, like components/debug or something like that?

import 'package:flame/components.dart';
import 'package:flame/text.dart';

class ComponentTrackComponent<T> extends TextComponent {
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit of a confusing name 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is so freaking confusing hahaha, but like I just submitted this as it was based on my perfomance debugging, and I was not really caring about names hahahahaha

Maybe ChildTrackComponent? Since this will track the number of a certain child of a parent?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ngl, the first time I read this I thought it was about a race car game! Since I interpreted "track" as the "race track" 😂

Copy link
Member

Choose a reason for hiding this comment

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

Maybe ChildTrackComponent?

I don't think that really conveys what it does either, maybe ChildCounterComponent or something like that?

Comment on lines 10 to 26
static final Map<String, int> _startTimes = {};
static final Map<String, int> _endTimes = {};

static void clear() {
_startTimes.clear();
_endTimes.clear();
}
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, do we really want all these to be static?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this is also part of the rough implementation. But at the same time, I thought that keeping then static would make the tracking be less impactful and make the tracking more accurate? But anyhow totally open to suggestion, at the time I just did it static because I needed something quick.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, if it is an explicit debug component it can probably just be kept as static, since debugging is supposed to be "quick & dirty" anyways.

@alestiago
Copy link
Contributor

I'm in favour of moving debug components to a debug subdirectory and maybe even prefixing all of them with Debug if that helps to avoid confusion.

@erickzanardo
Copy link
Member Author

I'm in favour of moving debug components to a debug subdirectory and maybe even prefixing all of them with Debug if that helps to avoid confusion.

hum, yeah, that is a fair argument, right now the fps component is also "loose" in the directory

@spydon
Copy link
Member

spydon commented Nov 10, 2023

I'm in favour of moving debug components to a debug subdirectory and maybe even prefixing all of them with Debug if that helps to avoid confusion.

I don't think we have to prefix every component with debug, but we could skip exporting them in components.dart and just export them in a separate debug.dart barrel file.

@erickzanardo erickzanardo marked this pull request as ready for review November 16, 2023 17:50
doc/flame/other/debug.md Outdated Show resolved Hide resolved
);
```

Will render the number of `SpriteAnimationComponent` that are children of the game `world`.
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to merge this with "So for example:" and put it above the code, since it sounds like it is continuing that sentence.

So for example, the following will render the number of SpriteAnimationComponent that are children of the game world:

@@ -48,6 +49,7 @@ export 'src/components/sprite_group_component.dart';
export 'src/components/text_box_component.dart';
export 'src/components/text_component.dart';
export 'src/components/text_element_component.dart';
export 'src/components/time_track_component.dart';
Copy link
Member

Choose a reason for hiding this comment

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

Move these to a debug.dart barrel file like we discussed :)

@@ -0,0 +1,65 @@
import 'dart:async';
Copy link
Member

Choose a reason for hiding this comment

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

Can you create a subdirectory inside component called debug too where these can live? And eventually we can move over other ones too, like the ones tracking fps (probably add those to the debug barrel file too).

@erickzanardo erickzanardo changed the title feat: Time track component feat: add TimeTrackComponent and ChildCounterComponent Nov 20, 2023
@erickzanardo erickzanardo changed the title feat: add TimeTrackComponent and ChildCounterComponent feat: dd TimeTrackComponent and ChildCounterComponent Nov 20, 2023
@erickzanardo erickzanardo changed the title feat: dd TimeTrackComponent and ChildCounterComponent feat: Add TimeTrackComponent and ChildCounterComponent Nov 20, 2023
@spydon spydon merged commit 6269551 into main Nov 21, 2023
10 checks passed
@spydon spydon deleted the feat/time-track-component branch November 21, 2023 07:19
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.

None yet

3 participants