Some issues:
Your type casting is not the most readable. Also, if you use the casted object more than once, it may be less inefficient.
Instead of:
if(object is Type)
((Type)object).DoSomething();
Use:
var casted = object as Type;
if (casted != null) {
casted.DoSomething();
}
This way, you do not need to cast the object every time it is used, so it may be faster, and it is more readable.
Also, for your components, it may be faster to use a Dictionary<Type, Component>, as it does not need to iterate through every key, so instead of:
if (_components.Find((c) => c is T) != null)
return;
You should use:
if (_components.ContainsKey(typeof(T))) {
return;
}
It is also a bad practice to exclude braces from if statements, as it may confuse some if there are multiple statements, all indented.
For example, you may write:
if (condition)
DoSomething();
DoSomethingElse();
At first, it may look perfectly normal, especially if you are doing a quick read-through. (Of course, using an auto-indenter solves the problem, but the code may still be hard to read for some.)
Furthermore, in this method:
public Entity[] GetChildren()
{
return _children.ToArray();
}
you are converting a List to an Array. Since List uses an Array internally, it is not expensive, but unless you have a good reason to keep it, it's better to leave it in list form. The same applies to GetComponents.
To answer your questionquestions:
This2. This should be good object-oriented code. The object-specific methods are defined on each object, and the Scene, which has a role of rendering the items, should be the one calling the Render method.
If it is intended for the children of CustomEEntity to be visible, it will be best to implement the Render method and Visible property in the CustomEntity.
On the other hand, if CustomEntity is not related to rendering, it should not have an IRenderable child at all. In this case, you may want to check the interfaces implemented by each child and component before adding, and make sure the object implements the interfaces as well.