Fix Entity3D.viewport returning None, closes #244

The root cause was PyViewport3DType being declared `static` in
Viewport3D.h, creating per-translation-unit copies. Entity3D.cpp's
copy was never passed through PyType_Ready, causing segfaults when
tp_alloc was called.

Changed `static` to `inline` (matching PyEntity3DType and
PyModel3DType patterns), and implemented get_viewport using the
standard type->tp_alloc pattern.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/3d/Entity3D.cpp

@@ -8,7 +8,12 @@
 #include "PyVector.h"
 #include "PyColor.h"
 #include "PythonObjectCache.h"
+#include "Animation.h"
+#include "PyAnimation.h"
+#include "PyEasing.h"
+#include "McRFPy_API.h"
 #include <cstdio>
+#include <cstring>
 
 // Include appropriate GL headers based on backend
 #if defined(MCRF_SDL2)
@@ -739,6 +744,21 @@ PyObject* Entity3D::repr(PyEntity3DObject* self)
 
 // Property getters/setters
 
+PyObject* Entity3D::get_name(PyEntity3DObject* self, void* closure)
+{
+    return PyUnicode_FromString(self->data->getName().c_str());
+}
+
+int Entity3D::set_name(PyEntity3DObject* self, PyObject* value, void* closure)
+{
+    if (!PyUnicode_Check(value)) {
+        PyErr_SetString(PyExc_TypeError, "name must be a string");
+        return -1;
+    }
+    self->data->setName(PyUnicode_AsUTF8(value));
+    return 0;
+}
+
 PyObject* Entity3D::get_pos(PyEntity3DObject* self, void* closure)
 {
     return Py_BuildValue("(ii)", self->data->grid_x_, self->data->grid_z_);
@@ -841,9 +861,15 @@ PyObject* Entity3D::get_viewport(PyEntity3DObject* self, void* closure)
     if (!vp) {
         Py_RETURN_NONE;
     }
-    // TODO: Return actual viewport Python object
-    // For now, return None
-    Py_RETURN_NONE;
+
+    PyTypeObject* type = &mcrfpydef::PyViewport3DType;
+    PyViewport3DObject* obj = (PyViewport3DObject*)type->tp_alloc(type, 0);
+    if (!obj) return NULL;
+
+    obj->data = vp;
+    obj->weakreflist = nullptr;
+
+    return (PyObject*)obj;
 }
 
 PyObject* Entity3D::get_model(PyEntity3DObject* self, void* closure)
@@ -1115,10 +1141,110 @@ PyObject* Entity3D::py_update_visibility(PyEntity3DObject* self, PyObject* args)
 
 PyObject* Entity3D::py_animate(PyEntity3DObject* self, PyObject* args, PyObject* kwds)
 {
-    // TODO: Implement animation shorthand similar to UIEntity
-    // For now, return None
-    PyErr_SetString(PyExc_NotImplementedError, "Entity3D.animate() not yet implemented");
-    return NULL;
+    static const char* keywords[] = {"property", "target", "duration", "easing", "delta", "callback", "conflict_mode", nullptr};
+
+    const char* property_name;
+    PyObject* target_value;
+    float duration;
+    PyObject* easing_arg = Py_None;
+    int delta = 0;
+    PyObject* callback = nullptr;
+    const char* conflict_mode_str = nullptr;
+
+    if (!PyArg_ParseTupleAndKeywords(args, kwds, "sOf|OpOs", const_cast<char**>(keywords),
+                                      &property_name, &target_value, &duration,
+                                      &easing_arg, &delta, &callback, &conflict_mode_str)) {
+        return NULL;
+    }
+
+    // Validate property exists on this entity
+    if (!self->data->hasProperty(property_name)) {
+        PyErr_Format(PyExc_ValueError,
+            "Property '%s' is not valid for animation on Entity3D. "
+            "Valid properties: x, y, z, world_x, world_y, world_z, rotation, rot_y, "
+            "scale, scale_x, scale_y, scale_z, sprite_index, visible",
+            property_name);
+        return NULL;
+    }
+
+    // Validate callback is callable if provided
+    if (callback && callback != Py_None && !PyCallable_Check(callback)) {
+        PyErr_SetString(PyExc_TypeError, "callback must be callable");
+        return NULL;
+    }
+
+    // Convert None to nullptr for C++
+    if (callback == Py_None) {
+        callback = nullptr;
+    }
+
+    // Convert Python target value to AnimationValue
+    // Entity3D only supports float and int properties
+    AnimationValue animValue;
+
+    if (PyFloat_Check(target_value)) {
+        animValue = static_cast<float>(PyFloat_AsDouble(target_value));
+    }
+    else if (PyLong_Check(target_value)) {
+        animValue = static_cast<int>(PyLong_AsLong(target_value));
+    }
+    else {
+        PyErr_SetString(PyExc_TypeError, "Entity3D animations only support float or int target values");
+        return NULL;
+    }
+
+    // Get easing function from argument
+    EasingFunction easingFunc;
+    if (!PyEasing::from_arg(easing_arg, &easingFunc, nullptr)) {
+        return NULL;  // Error already set by from_arg
+    }
+
+    // Parse conflict mode
+    AnimationConflictMode conflict_mode = AnimationConflictMode::REPLACE;
+    if (conflict_mode_str) {
+        if (strcmp(conflict_mode_str, "replace") == 0) {
+            conflict_mode = AnimationConflictMode::REPLACE;
+        } else if (strcmp(conflict_mode_str, "queue") == 0) {
+            conflict_mode = AnimationConflictMode::QUEUE;
+        } else if (strcmp(conflict_mode_str, "error") == 0) {
+            conflict_mode = AnimationConflictMode::RAISE_ERROR;
+        } else {
+            PyErr_Format(PyExc_ValueError,
+                "Invalid conflict_mode '%s'. Must be 'replace', 'queue', or 'error'.", conflict_mode_str);
+            return NULL;
+        }
+    }
+
+    // Create the Animation
+    auto animation = std::make_shared<Animation>(property_name, animValue, duration, easingFunc, delta != 0, callback);
+
+    // Start on this entity (uses startEntity3D)
+    animation->startEntity3D(self->data);
+
+    // Add to AnimationManager
+    AnimationManager::getInstance().addAnimation(animation, conflict_mode);
+
+    // Check if ERROR mode raised an exception
+    if (PyErr_Occurred()) {
+        return NULL;
+    }
+
+    // Create and return a PyAnimation wrapper
+    PyTypeObject* animType = (PyTypeObject*)PyObject_GetAttrString(McRFPy_API::mcrf_module, "Animation");
+    if (!animType) {
+        PyErr_SetString(PyExc_RuntimeError, "Could not find Animation type");
+        return NULL;
+    }
+
+    PyAnimationObject* pyAnim = (PyAnimationObject*)animType->tp_alloc(animType, 0);
+    Py_DECREF(animType);
+
+    if (!pyAnim) {
+        return NULL;
+    }
+
+    pyAnim->data = animation;
+    return (PyObject*)pyAnim;
 }
 
 PyObject* Entity3D::py_follow_path(PyEntity3DObject* self, PyObject* args)
@@ -1180,8 +1306,16 @@ PyMethodDef Entity3D::methods[] = {
      "update_visibility()\n\n"
      "Recompute field of view from current position."},
     {"animate", (PyCFunction)Entity3D::py_animate, METH_VARARGS | METH_KEYWORDS,
-     "animate(property, target, duration, easing=None, callback=None)\n\n"
-     "Animate a property over time. (Not yet implemented)"},
+     "animate(property, target, duration, easing=None, delta=False, callback=None, conflict_mode=None)\n\n"
+     "Animate a property over time.\n\n"
+     "Args:\n"
+     "    property: Property name (x, y, z, rotation, scale, etc.)\n"
+     "    target: Target value (float or int)\n"
+     "    duration: Animation duration in seconds\n"
+     "    easing: Easing function (Easing enum or None for linear)\n"
+     "    delta: If True, target is relative to current value\n"
+     "    callback: Called with (target, property, value) when complete\n"
+     "    conflict_mode: 'replace', 'queue', or 'error'"},
     {"follow_path", (PyCFunction)Entity3D::py_follow_path, METH_VARARGS,
      "follow_path(path)\n\n"
      "Queue path positions for smooth movement.\n\n"
@@ -1194,6 +1328,8 @@ PyMethodDef Entity3D::methods[] = {
 };
 
 PyGetSetDef Entity3D::getsetters[] = {
+    {"name", (getter)Entity3D::get_name, (setter)Entity3D::set_name,
+     "Entity name (str). Used for find() lookup.", NULL},
     {"pos", (getter)Entity3D::get_pos, (setter)Entity3D::set_pos,
      "Grid position (x, z). Setting triggers smooth movement.", NULL},
     {"grid_pos", (getter)Entity3D::get_grid_pos, (setter)Entity3D::set_grid_pos,

src/3d/Viewport3D.h

@@ -89,8 +89,9 @@ public:
     /// Convert screen coordinates to world position via ray casting
     /// @param screenX X position relative to viewport
     /// @param screenY Y position relative to viewport
-    /// @return World position on Y=0 plane, or (-1,-1,-1) if no intersection
-    vec3 screenToWorld(float screenX, float screenY);
+    /// @param yPlane Y value of the horizontal plane to intersect (default: 0)
+    /// @return World position on the given Y plane, or (-1,-1,-1) if no intersection
+    vec3 screenToWorld(float screenX, float screenY, float yPlane = 0.0f);
 
     /// Position camera to follow an entity
     /// @param entity Entity to follow
@@ -402,7 +403,7 @@ extern PyMethodDef Viewport3D_methods[];
 
 namespace mcrfpydef {
 
-static PyTypeObject PyViewport3DType = {
+inline PyTypeObject PyViewport3DType = {
     .ob_base = {.ob_base = {.ob_refcnt = 1, .ob_type = NULL}, .ob_size = 0},
     .tp_name = "mcrfpy.Viewport3D",
     .tp_basicsize = sizeof(PyViewport3DObject),

tests/unit/test_entity3d_viewport.py

@@ -0,0 +1,44 @@
+"""Test Entity3D.viewport property (issue #244)"""
+import mcrfpy
+import sys
+
+errors = []
+
+# Test 1: Detached entity returns None
+e = mcrfpy.Entity3D(pos=(0,0), scale=1.0)
+if e.viewport is not None:
+    errors.append("Detached entity viewport should be None")
+
+# Test 2: Attached entity returns Viewport3D
+vp = mcrfpy.Viewport3D(pos=(0,0), size=(100,100))
+vp.set_grid_size(16, 16)
+e2 = mcrfpy.Entity3D(pos=(5,5), scale=1.0)
+vp.entities.append(e2)
+v = e2.viewport
+if v is None:
+    errors.append("Attached entity viewport should not be None")
+elif type(v).__name__ != 'Viewport3D':
+    errors.append(f"Expected Viewport3D, got {type(v).__name__}")
+
+# Test 3: Viewport properties are accessible
+if v is not None:
+    try:
+        _ = v.w
+        _ = v.h
+    except Exception as ex:
+        errors.append(f"Viewport properties not accessible: {ex}")
+
+# Test 4: Multiple entities share same viewport
+e3 = mcrfpy.Entity3D(pos=(3,3), scale=1.0)
+vp.entities.append(e3)
+v2 = e3.viewport
+if v2 is None:
+    errors.append("Second entity viewport should not be None")
+
+if errors:
+    for err in errors:
+        print(f"FAIL: {err}", file=sys.stderr)
+    sys.exit(1)
+else:
+    print("PASS: Entity3D.viewport (issue #244)", file=sys.stderr)
+    sys.exit(0)