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

Clean up profiler #37966

Merged
merged 1 commit into from
Apr 10, 2021
Merged

Clean up profiler #37966

merged 1 commit into from
Apr 10, 2021

Conversation

pkowal1982
Copy link
Contributor

  • Fix issue where you can't select by mouse all frames (for example frame 0 can be selected only by edit field)
  • Disable frame edit field when no data available
  • Disable clear button when nothing to clean
  • Align cursor to the middle of selected frame (it was misplaced)
  • Remove some dead code

@Calinou
Copy link
Member

Calinou commented Mar 30, 2021

@pkowal1982 Could you look into rebasing this pull request on the latest master branch?

@pkowal1982
Copy link
Contributor Author

@Calinou yeah, I'll try to look into it before this weekend.

@pkowal1982 pkowal1982 requested a review from a team as a code owner April 1, 2021 17:28
@pkowal1982 pkowal1982 force-pushed the cleanup-profiler branch 2 times, most recently from 8f521b3 to d1cd56f Compare April 1, 2021 22:53
@pkowal1982
Copy link
Contributor Author

@Calinou Done.

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Haven't tried the changes yet, but see comments on style (we always require braces in control flows)

void EditorProfiler::add_frame_metric(const Metric &p_metric, bool p_final) {
++last_metric;
if (last_metric >= frame_metrics.size()) {
last_metric = 0;
}

total_metrics++;
if (total_metrics > frame_metrics.size())
total_metrics = frame_metrics.size();
Copy link
Collaborator

Choose a reason for hiding this comment

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

	if (total_metrics > frame_metrics.size()) {
		total_metrics = frame_metrics.size();
	}

cursor_metric_edit->set_min(_get_frame_metric(0).frame_number);

if (!seeking)
cursor_metric_edit->set_value(p_metric.frame_number);
Copy link
Collaborator

Choose a reason for hiding this comment

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

	if (!seeking) {
		cursor_metric_edit->set_value(p_metric.frame_number);
	}

@pkowal1982 pkowal1982 requested a review from Faless April 6, 2021 18:46
Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

I've tested it a bit, it's a great improvement.
There are still issues when you select a frame and then close the game without resuming it (the play/stop button stays disabled), but that's already happening in master so we can address that in a separate PR.
Great job 🏆 !

@Faless Faless merged commit d540875 into godotengine:master Apr 10, 2021
@pkowal1982 pkowal1982 deleted the cleanup-profiler branch October 29, 2022 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants