Add field tags#357
Draft
KrishaDeshkool wants to merge 3 commits intoeclipse-score:mainfrom
Draft
Conversation
64ca029 to
323edba
Compare
The new order allows us to set the bindings to nullptrs by default for test only constructors. This will be useful for fields which will require optionally inserting get / set bindings. We change all constructors so that they're all consistent.
- Use a single test constructor which creates the get / set methods according to the provided bindings. This allows us to remove the test-only constructor overloads for each template arg combination. - Always store the get / set method dispatches as unique_ptrs even if they're not enabled (when disabled, they'll simply be nullptrs). This allows us to have a single private constructor which always accepts unique_ptrs (which may be valid or nullptrs).
323edba to
278adac
Compare
278adac to
c034c1e
Compare
Replaces the EnableGet/EnableSet/EnableNotifier bool template parameters on ProxyField and SkeletonField with a variadic pack of tags (WithGetter, WithSetter) declared in field_tags.h. Presence of a tag enables the matching API. The notifier surface stays unconditional on both sides, since a field without a notifier cannot propagate value changes. Also updates friend/forward declarations in ProxyEvent, ProxyMethod, SkeletonEvent and SkeletonMethod, the Trait::Field aliases plus the traits example doc, and the skeleton_field_test.cpp call sites.
Contributor
Author
|
Once the CI is green, i will make this PR open and can we reviewed. |
c034c1e to
894a5e7
Compare
crimson11
requested changes
Apr 28, 2026
| template <bool ES = EnableSet, typename std::enable_if<ES, int>::type = 0, typename CallableType> | ||
| /// Registers a handler invoked when a proxy issues the field setter. | ||
| /// The handler receives the proxy-requested value by reference and may modify it in-place; | ||
| /// the (possibly modified) value is then broadcast via Update(). |
Contributor
There was a problem hiding this comment.
See teams-chat:
the (possibly modified) value is then broadcast via Update().
-> the (possibly modified) value is then stored as the "latest value" via call to SkeletonField::Update().
| IsSetHandlerRegisteredType is_set_handler_registered_{}; | ||
|
|
||
| // EnableSet=true: checks the flag; EnableSet=false: no setter, no handler required. | ||
| bool IsSetHandlerRegistered() const noexcept override |
Contributor
There was a problem hiding this comment.
SFINAE out if no WithSetter tag exists. -> RegisterSetHandler()
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.