-
-
Notifications
You must be signed in to change notification settings - Fork 215
[feat] extend the components class to add json/serialize/deserialize to component properties #3617
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
base: main
Are you sure you want to change the base?
Conversation
|
before merging, would need documentation and testing updated, but its too early for that, i need people to look at this, IMHO its not a trivial change, although it doesn't break anything active |
Deploying excaliburjs with
|
| Latest commit: |
9d6baee
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://395f7a56.excaliburjs.pages.dev |
| Branch Preview URL: | https://serializablecomponents.excaliburjs.pages.dev |
I haven't deep dived the code yet, but a few high level comments. Looking really good! I could see a case for There is also potential for decorators for serializable attributes. But I've always been on the fence about decorators because it adds an undesirable level of indirection. An example might be: class BodyComponent extends Component implements Clonable<BodyComponent>, Serializable<BodyComponent> {
@serializable
friction: number;
@serializable
mass: number;
}Actually, even writing that out now, I don't think decorators would be a good fit. Btw, you don't know how hard it is for me to write |
|
@chrisk-7777 i don't hate that at all. I'm doing some research, because i'm trying to understand the patterns with implements Clonable, as the base Component class has clone on it..... reads stuff.... comes back... So, my question is then... why is the base class for clone() already defined on the Component class. Wouldn't the extend Component take care of that already? I'm asking out of ignorance here... |
Yea, so there is a difference between I will butcher this, its a huge topic, so I'm just cherry picking the absolute critical parts for this convo.
The key difference for this convo is with interfaces (the This probably explains it better than I could: https://stackoverflow.com/questions/38834625/whats-the-difference-between-extends-and-implements-in-typescript In the case of this PR, you could have an array of objects - some that do implement Serializable and some that don't. export function isSerializable(obj: any): obj is Serializable {
return (
obj != null &&
typeof obj.serialize === "function" &&
typeof obj.deserialize === "function"
);
}
for (const item of arr) {
if (isSerializable(item)) {
// we're both compile-time safe and runtime safe at this point
console.log("Serializable:", item.serialize());
} else {
console.log("Not serializable - skip!", item);
}
}Another key diff in Typescript is a class may implement multiple interfaces but extend only one parent. (Interestingly some languages allow extending multiple, but that would be chaos.). Again the actor is a great example of leveraging this: https://github.com/excaliburjs/Excalibur/blob/main/src/engine/Actor.ts#L266 So in the example above |
|
....and I missed your core question.
public clone(): BodyComponent {
const component = super.clone() as BodyComponent;
return component;
}This is just calling the parent This example is probably more confusing, because we're mixing You could omit Whew, I'm convinced that isn't explained well. In the case of |
|
that's a solid explanation, @chrisk-7777 . Thx,.... my only concern with forcing an implementation, is that i'm putting a 'default' serialize() and deserialize() on the component class, i suspect MOST custom components could just use the default. but complicated components 'can' override it with a custom serializer |
Ah yep, that totally makes sense, esp. when there is a lot of shared functionality 👍 |
|
I gonna re-attempt this concept tomorrow as a centralized 'serializer' for all of Ex as opposed to shoving this functionality into Entity and Components... getting a ton of circular arguements |
|
Got the vitest spec file done tonight and pushed to the PR |
…xed linting and test issues
|
@eonarheim this has been open awhile, is this something we're interested in? |
|
My bad, I'll review ASAP |
This PR adds comprehensive serialization and deserialization capabilities to Excalibur's core engine components, enabling persistent storage and restoration of component state.
Changes Made
Core Component Serialization Framework
Added
serialize()Added
deserialize()Added
toJSON()added private helper methods
BodyComponent required a custom serialization method that overrides the default
ColliderComponent required a custom serialization method that overrides the default
GraphicsComponent required a custom serialization method that overrides the default
TransformComponent required a custom serialization method that overrides the default
Benefits
Cursory testing locally was favorable, but I busted this out i a vacuum
tested with all native Actor components, some could use default serialization
tested with custom component