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

Refactor SlotMap and Slot classes #896

Merged
merged 5 commits into from May 21, 2021
Merged

Refactor SlotMap and Slot classes #896

merged 5 commits into from May 21, 2021

Conversation

gbrail
Copy link
Collaborator

@gbrail gbrail commented May 17, 2021

Ensure that Slot and all related classes are separate classes now and not just nested inside ScriptableObject.

Also, break the two Slot implementations (regular and "GetterSlot") into four so we can use inheritance rather than complex "instanceof" logic to implement the different types of slots.

This simplifies the structure and logic and will make it easier to extend stuff later.

Ensure that Slot and all related classes are separate classes now and
not just nested inside ScriptableObject.

Also, break the two Slot implementations (regular and "GetterSlot") into
four so we can use inheritance rather than complex "instanceof" logic
to implement the different types of slots.

This will be helpful as I'd like to introduce a lambda-based Slot and it was
too much complexity the old way.
Copy link
Collaborator

@rbri rbri left a comment

Choose a reason for hiding this comment

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

looks like a real improvement, did not found the time to dive into the details - but one suggestion:
What about placing all the different slot implementations in an separate sub package?

@gbrail
Copy link
Collaborator Author

gbrail commented May 21, 2021

I'm going to experiment with a different package -- but it's going to make this an even bigger PR.

@gbrail
Copy link
Collaborator Author

gbrail commented May 21, 2021

Moving the package is going to result in a ton of changes because of use of package-protected fields and methods. I'm going to leave these classes in the main package and we might consider a change at a future time.

@gbrail gbrail merged commit 2008a1a into master May 21, 2021
@gbrail gbrail deleted the slot-map-refactor branch May 21, 2021 20:42
@p-bakker p-bakker added this to the Release 1.7.14 milestone Oct 13, 2021
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.

None yet

4 participants