Fix drag'n'drop of selected event invalidating the event in memory (potential crash)

.vscode/settings.json

@@ -83,7 +83,8 @@
         "__threading_support": "cpp",
         "any": "cpp",
         "array": "cpp",
-        "cinttypes": "cpp"
+        "cinttypes": "cpp",
+        "numeric": "cpp"
     },
     "files.exclude": {
         "Binaries/*build*": true,

Core/GDCore/Events/EventsList.cpp

@@ -99,6 +99,23 @@ bool EventsList::Contains(const gd::BaseEvent& eventToSearch,
   return false;
 }
 
+bool EventsList::MoveEventToAnotherEventsList(const gd::BaseEvent& eventToMove,
+                                  gd::EventsList& newEventsList,
+                                  std::size_t newPosition) {
+
+  for (std::size_t i = 0; i < GetEventsCount(); ++i) {
+    if (events[i].get() == &eventToMove) {
+      std::shared_ptr<BaseEvent> event = events[i];
+      events.erase(events.begin() + i);
+
+      newEventsList.InsertEvent(event, newPosition);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 EventsList::EventsList(const EventsList& other) { Init(other); }
 
 EventsList& EventsList::operator=(const EventsList& other) {

Core/GDCore/Events/EventsList.h

@@ -52,7 +52,8 @@ class GD_CORE_API EventsList {
    * \brief Insert the specified event to the list.
    * \note The event passed by parameter is not copied.
    * \param event The smart pointer to the event that must be inserted into the
-   * list \param position Insertion position. If the position is invalid, the
+   * list
+   * \param position Insertion position. If the position is invalid, the
    * object is inserted at the end of the objects list.
    */
   void InsertEvent(std::shared_ptr<gd::BaseEvent> event,
@@ -142,6 +143,25 @@ class GD_CORE_API EventsList {
    */
   bool Contains(const gd::BaseEvent& eventToSearch,
                 bool recursive = true) const;
+
+  /**
+   * Move the specified event, that must be in the events list, to another
+   * events list *without* invalidating the event (i.e: without
+   * destroying/cloning it) in memory.
+   *
+   * \warning newEventsList is supposed not to be contained inside the event
+   * (you should not try
+   * to move an event inside one of its children/grand children events).
+   *
+   * \param eventToMove The event to be moved
+   * \param newEventsList The new events list
+   * \param newPosition The position in the new events list
+   * \return true if the move was made, false otherwise (for example, if
+   * eventToMove is not found in the list)
+   */
+  bool MoveEventToAnotherEventsList(const gd::BaseEvent& eventToMove,
+                                    gd::EventsList& newEventsList,
+                                    std::size_t newPosition);
   ///@}
 
   /** \name std::vector API compatibility

GDevelop.js/Bindings/Bindings.idl

@@ -1272,6 +1272,7 @@ interface EventsList {
     void RemoveEvent([Const, Ref] BaseEvent event);
     unsigned long GetEventsCount();
     boolean Contains([Const, Ref] BaseEvent event, boolean recursive);
+    boolean MoveEventToAnotherEventsList([Const, Ref] BaseEvent eventToMove, [Ref] EventsList newEventsList, unsigned long newPosition);
     boolean IsEmpty();
     void Clear();
 

GDevelop.js/__tests__/Core.js

@@ -1910,6 +1910,34 @@ describe('libGD.js', function() {
 
       list.delete();
     });
+
+    it('can move an event to another list without invalidating it/copying it in memory', function() {
+      var list1 = new gd.EventsList();
+      var list2 = new gd.EventsList();
+
+      var list1ParentEvent = list1.insertEvent(new gd.StandardEvent(), 0);
+      var list2ParentEvent = list2.insertEvent(new gd.StandardEvent(), 0);
+      var originalSubEvent = list1ParentEvent
+        .getSubEvents()
+        .insertEvent(new gd.StandardEvent(), 0);
+      var originalSubEventPtr = originalSubEvent.ptr;
+
+      expect(
+        list1ParentEvent
+          .getSubEvents()
+          .moveEventToAnotherEventsList(
+            originalSubEvent,
+            list2ParentEvent.getSubEvents(),
+            0
+          )
+      ).toBe(true);
+      expect(list2ParentEvent.getSubEvents().getEventsCount()).toBe(1);
+      var movedSubEvent = list2ParentEvent.getSubEvents().getEventAt(0);
+      expect(movedSubEvent.ptr).toBe(originalSubEventPtr);
+
+      list1.delete();
+      list2.delete();
+    });
   });
 
   describe('gd.BaseEvent', function() {
@@ -2902,9 +2930,7 @@ describe('libGD.js', function() {
     });
   });
 
-
   describe('gd.ParameterMetadataTools', function() {
-
     it('can create an object container from parameters', function() {
       const project = gd.ProjectHelper.createNewGDJSProject();
 
@@ -3132,12 +3158,16 @@ describe('libGD.js', function() {
 
       expect(instructionMetadata.getParametersCount()).toBe(0);
       instructionMetadata.addParameter('type', 'label', '', false);
-      instructionMetadata.setParameterLongDescription("Blabla");
+      instructionMetadata.setParameterLongDescription('Blabla');
       expect(instructionMetadata.getParametersCount()).toBe(1);
       expect(instructionMetadata.getParameter(0).getType()).toBe('type');
-      expect(instructionMetadata.getParameter(0).getDescription()).toBe('label');
-      expect(instructionMetadata.getParameter(0).getLongDescription()).toBe('Blabla');
-    })
+      expect(instructionMetadata.getParameter(0).getDescription()).toBe(
+        'label'
+      );
+      expect(instructionMetadata.getParameter(0).getLongDescription()).toBe(
+        'Blabla'
+      );
+    });
   });
 
   describe('gd.ExpressionMetadata', () => {
@@ -3146,11 +3176,13 @@ describe('libGD.js', function() {
 
       expect(expressionMetadata.getParametersCount()).toBe(0);
       expressionMetadata.addParameter('type', 'label', '', false);
-      expressionMetadata.setParameterLongDescription("Blabla");
+      expressionMetadata.setParameterLongDescription('Blabla');
       expect(expressionMetadata.getParametersCount()).toBe(1);
       expect(expressionMetadata.getParameter(0).getType()).toBe('type');
       expect(expressionMetadata.getParameter(0).getDescription()).toBe('label');
-      expect(expressionMetadata.getParameter(0).getLongDescription()).toBe('Blabla');
-    })
+      expect(expressionMetadata.getParameter(0).getLongDescription()).toBe(
+        'Blabla'
+      );
+    });
   });
 });

newIDE/app/src/EventsSheet/EventsTree/index.js

@@ -360,10 +360,11 @@ export default class ThemableEventsTree extends Component<EventsTreeProps, *> {
     const { event, eventsList } = node;
 
     // Do the move
-    const newEvent = event.clone();
-    eventsList.removeEvent(event);
-    targetEventsList.insertEvent(newEvent, targetPosition);
-    newEvent.delete();
+    // Note that moveEventToAnotherEventsList does not invalidate the
+    // references to the event in memory - so things refering to this event like the
+    // selection in EventsSheet remain valid. This might not be needed anymore
+    // if events drag'n'drop is reworked to be similar to instructions drag'n'drop.
+    eventsList.moveEventToAnotherEventsList(event, targetEventsList, targetPosition);
 
     this.forceEventsUpdate();
     this.props.onEventMoved();