search

GameDevTecnico / cubos

A still very barebones game engine focused on voxels and data-oriented programming
https://gamedevtecnico.github.io/cubos/
MIT License
83 stars 23 forks source link

Implement metrics #1153

Closed roby2014 closed 2 months ago

roby2014 commented 4 months ago

Description

Check the sample for usage

TLDR: The goal is to have a simple but extendible core class to register, search and debug metrics, with a compiler option to enable it

Checklist

github-actions[bot] commented 4 months ago

PR Preview Action v1.4.7 :---: :rocket: Deployed preview to https://GameDevTecnico.github.io/cubos/docs-preview/pr-1153/ on branch gh-pages at 2024-06-12 18:11 UTC

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 40.70%. Comparing base (da70041) to head (1a4000a).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1153 +/- ## ========================================== + Coverage 40.48% 40.70% +0.22% ========================================== Files 352 354 +2 Lines 26173 26272 +99 ========================================== + Hits 10596 10695 +99 Misses 15577 15577 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

RiscadoA commented 3 months ago

Since this PR is still blocked on the other one, I'm marking it as a draft. Does it really depend on the other PR though? They seem mostly unrelated.

roby2014 commented 3 months ago

It does not depend anymore yeah, you're right.

roby2014 commented 3 months ago

Eh no idea why static analysis is failing

RiscadoA commented 3 months ago

Eh no idea why static analysis is failing

It's an error in the action. It's trying to dismiss a review which no longer exists and crashes. We can safely ignore it. TL;DR: not a problem in your code. @joaomanita @diogomsmiranda pls review when you have the chance.

diogomsmiranda commented 3 months ago

I also didn't understand the need to make such a sample basically copying from the test. Shouldn't we make something more in tune with what would be a real example?

RiscadoA commented 3 months ago

I also didn't understand the need to make such a sample basically copying from the test. Shouldn't we make something more in tune with what would be a real example?

Agree, we should avoid this, could you improve it @roby2014?

diogomsmiranda commented 3 months ago

I would rather have the metrics to be explicitly generated with different names and contexts. But if @RiscadoA is ok with a sample like this It's not a deal breaker for me either.

roby2014 commented 3 months ago

I would rather have the metrics to be explicitly generated with different names and contexts. But if @RiscadoA is ok with a sample like this It's not a deal breaker for me either.

Can you give me an example? What should I sample?

diogomsmiranda commented 2 months ago

I also didn't understand the need to make such a sample basically copying from the test. Shouldn't we make something more in tune with what would be a real example?

@roby2014 thank you for addressing this! LGTM

RiscadoA commented 2 months ago

You can ignore the failing lint check btw, feel free to just press merge