stride
stride copied to clipboard
Added overload for Entity constructor
PR Details
Added overload for Entity constructor.
Description
Now you can create entities with rotation parameter
Motivation and Context
In my game with procedural generation, many prefabs of rooms are created. I wanted them to randomly rotate, but I did not find a parameter with rotation.
Types of changes
- [ ] Docs change / refactoring / dependency upgrade
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist
- [x] My change requires a change to the documentation.
- [ ] I have added tests to cover my changes.
- [ ] All new and existing tests passed.
Hmm, I don't know if that's the best move here - the next person will want a position and a scale and another all three - so maybe there's a way to just have one constructor with optional parameters? We could break binary compatibility here and modify the constructor that currently takes only position.
My main issue with it is how easy it is for the user to write a wrapper method that would handle setting the rotation on an entity without modifying the source code.
Maybe we could have an experimental/preview quality of life extension(s), and if we knew that people are using some of them heavily and frequently, and it would be highly requested (we could vote after certain period), then it could move to the core..
But for this, would be great to have some docs so these are listed and visible as experimental?
position and a scale and another all three
Good point, it should also have the scale parameter as an optional parameter set to 1. then you got one with position and one with all three. that should do... if it is possible to add the two parameters to the original constructor without breaking changes, I'd be for that.
Maybe we could have an experimental/preview quality of life extension(s)
This exists here, now that you mention it, I believe it has all that already: https://github.com/dfkeenan/StrideToolkit
In general I would be against such a change but for this particular case I think it makes sense, setting an initial rotation for an entity is a fairly common operation, I do think that this overload is worth the very small amount of bloat it introduces.
We could repurpose the second constructor for this though;
public Entity(string name)
: this(Vector3.Zero, name)
{
}
to this
public Entity(string name = null, Vector3 position = default, Quaternion? rotation = null, Vector3? scale = null)
: this(name, false)
{
Id = Guid.NewGuid();
TransformValue = new TransformComponent { Position = position, Rotation = rotation ?? Quaternion.Identity, Scale = scale ?? Vector3.One };
Components.Add(TransformValue);
}
Users would then pick and choose what they want with something like the following
new Entity(rotation:Quaternion.RotationY(180f));
new Entity("Some Entity", scale:Vector3.One * 2f);
new Entity("Some Entity", new Vector3(1f, 2f, 3f), Quaternion.RotationY(180f));
What do you think ?
I have got a few side questions (probably missing in our docs):
- Is it a good practise to name the entities?
- When you would name them?
- When you wouldn't name them?
I would use your input for our docs. Thanks :)
Hey @VaclavElias, this is a bit off topic, let's go over that on discord. I'll hide your message to avoid derailing the conversation.
@palkercode would be up for updating this PR with Eideren's suggestion in https://github.com/stride3d/stride/pull/1499#issuecomment-1223764151 ?