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

[Core]: add control function references #353

Closed

Conversation

GwnDaan
Copy link
Member

@GwnDaan GwnDaan commented Nov 22, 2023

Hmm, this basically should allow us to use weak_ptr's more easily throughout the stack for (Partnered)ControlFunction classes. Because of the way we assign null_ptrs to shared_ptr's when we want to indicate no specific control function, this also meant that you don't know if the weak_ptr expired or that it has the "no specific control function" assigned.

This is only needed if we still want to continue the journey of destroying a ControlFunction in one place, and then automatically stopping all functionality that depended upon that CF (I think we do want that? not sure tbh)

@GwnDaan GwnDaan requested a review from ad3154 November 22, 2023 13:51
@GwnDaan GwnDaan self-assigned this Nov 22, 2023
@GwnDaan
Copy link
Member Author

GwnDaan commented Nov 22, 2023

Curious on your thoughts @ad3154, I took a few shortcuts here and there to get it working such that I could get your feedback, such as implicit conversions

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

87.6% 87.6% Coverage
0.0% 0.0% Duplication

@ad3154
Copy link
Member

ad3154 commented Nov 22, 2023

I think we do want that?

Hmm, I'm not sure... I'm not really sure what to say I guess. I'm not sure how much people are creating and deleting partners during the lifetime of their application. I would think this would be a relatively rare thing to do, as if you want to dynamically lock onto and remove numerous partners at runtime, you'd probably not want to even use a partner for most of those cases, and instead just register for PGNs from everyone, and use the regular ControlFunction instead? That's how our VT server class works I guess. Perhaps making the user clean up all references to the shared pointer of a partner instead forces them to not have stale client/interface objects as well, encouraging them to delete their VT client and whatnot instead of letting them hang around with a dead ControlFunctionReference, IDK

@GwnDaan
Copy link
Member Author

GwnDaan commented Nov 22, 2023

Yeah well f*ck hahah. I agree, I jumped in too soon I think. I feel like many things are not absolutely perfect yet, but then when I try to change something, I guess it starts out really promising but in the end it's also not optimal 😦

@GwnDaan GwnDaan closed this Nov 22, 2023
@GwnDaan
Copy link
Member Author

GwnDaan commented Nov 22, 2023

Though as a positive note, at least I learned some things about implicited/explicit conversions today😄

@GwnDaan GwnDaan deleted the daan/more-robust-cf-addresses branch November 23, 2023 18:37
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.

2 participants