Skip to content

feat: add registry base class and opaque handle concepts#15

Merged
gituser12981u2 merged 4 commits into
alex_stufffrom
chore/registry_infrastructure
Jun 3, 2026
Merged

feat: add registry base class and opaque handle concepts#15
gituser12981u2 merged 4 commits into
alex_stufffrom
chore/registry_infrastructure

Conversation

@gituser12981u2

@gituser12981u2 gituser12981u2 commented May 31, 2026

Copy link
Copy Markdown
Owner

Implemented the RFC-0001 and RFC-0002 (#11 and #14).

This is implemented in the registry_base.hpp, registry_concepts.hpp, and opaque_handle.hpp all under include/engine.

I still need to properly comment these with DOxygen.

I also update frame registry to use the registry base class and deleted device and instance registry/handles since they are not needed for such low level objects, probably.

@gituser12981u2 gituser12981u2 requested a review from alexcu2718 May 31, 2026 20:06
@gituser12981u2 gituser12981u2 self-assigned this May 31, 2026
@gituser12981u2 gituser12981u2 added the enhancement New feature or request label May 31, 2026
Comment thread include/quark/vk/frame/details/frame_registry.hpp
Comment thread include/quark/engine/registry/registry_concepts.hpp
@alexcu2718

alexcu2718 commented Jun 2, 2026

Copy link
Copy Markdown
Collaborator

So many deleted lines!!!~~~~~~!!!!

While going through the deleted lines, it became clearer to me that we should revisit some of our naming conventions.
For class/struct members, should we consider prefixing them with their responsibility? For example:

// Before
struct SpdlogFileConfig {
  std::string_view file_path = "quark.log";
  std::size_t max_bytes = 50ULL * 1024ULL * 1024ULL;
  std::size_t max_files = 5;
};


// After
struct SpdlogFileConfig {
  std::string_view log_file_path = "quark.log"; //probably too verbose, maybe just l_*? 
  std::size_t log_max_bytes = 50ULL * 1024ULL * 1024ULL;
  std::size_t log_max_files = 5;
};

I believe this makes it a bit clearer what's doing what.

@alexcu2718 alexcu2718 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Overall a lot more clarity on this.

ALSO SCREW YOU FOR MESSING UP MY NEXT PR YOU KNOW I HATE MERGE CONFLICTS.

@gituser12981u2

gituser12981u2 commented Jun 2, 2026

Copy link
Copy Markdown
Owner Author

Overall a lot more clarity on this.

ALSO SCREW YOU FOR MESSING UP MY NEXT PR YOU KNOW I HATE MERGE CONFLICTS.

rebase than kys. Also thanks cutie

@gituser12981u2

Copy link
Copy Markdown
Owner Author

So many deleted lines!!!~~~~~~!!!!

While going through the deleted lines, it became clearer to me that we should revisit some of our naming conventions. For class/struct members, should we consider prefixing them with their responsibility? For example:

// Before
struct SpdlogFileConfig {
  std::string_view file_path = "quark.log";
  std::size_t max_bytes = 50ULL * 1024ULL * 1024ULL;
  std::size_t max_files = 5;
};


// After
struct SpdlogFileConfig {
  std::string_view log_file_path = "quark.log"; //probably too verbose, maybe just l_*? 
  std::size_t log_max_bytes = 50ULL * 1024ULL * 1024ULL;
  std::size_t log_max_files = 5;
};

I believe this makes it a bit clearer what's doing what.

I think that is probably a good idea for some ambiguous fields. I feel like in this specific instance it's quite clear when one writes something like SpdlogFileConfig.file_path. If we had multiple file paths, which we might in the future, then maybe a rename would be good, but I don't think it should be a hard and fast rule to always prefix class/struct members with something. Not to sure though.

@gituser12981u2 gituser12981u2 merged commit 21de807 into alex_stuff Jun 3, 2026
10 checks passed
@gituser12981u2 gituser12981u2 deleted the chore/registry_infrastructure branch June 3, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants