Comments (11)
Some news on this bug ? Seems that a gd::Instruction contained inside list is invalid (causing i->GetType() failure).
from gdevelop.
The bug happen when dragging an instruction from a list into a OR/AND/NOT condition from the same list: as removing the dragged condition implies that the list is changed, every instruction are changed and so the pointer to the list where the condition is dropped become invalid..
The problem does not happen with events as they are managed by (shared) pointers and not by value (so any reference to an event remains valid even if it is moved).
Maybe I should prevent any drag'n'drop to a subcondition list for now.
from gdevelop.
Or return NULL instead of the list.
from gdevelop.
Should we convert instructions lists to instruction's pointers lists ? (Is this easy to do ?)
from gdevelop.
Well, instead of having list of instructions represented as std::vector < gd::InstructionItem >
we should have an gd::InstructionsList
class (just like we manage events using gd::EventsList
).
Internally, this class will manage instructions in a std::vector but not by value as now but using smart pointers (std::shared_ptr<Instruction>
) => this enables to swap/move/reorder instructions in the list (or even move instructions from list to another list) without losing the reference to it.
Currently, as they are stored by value, any change in a list has the potential to invalidate all the list (for example if the list is reallocated) so we can't keep any pointer (or iterator) on an element of the list.
This is fine when you're just dealing with list of things: do a copy of whatever you want to move, and swap/move it using the index. Just don't keep pointers on the Instruction
because any change in the vector can trigger reallocation and invalidate all pointers or iterators.
But in GDevelop instructions can have subinstructions, so in fact we are not dealing with simple arrays/lists but with a tree of instructions. If you try to move an instruction into a list of a subinstructions, you potentially risk, when you add/remove instructions, to invalidate all the pointers.
This is what is happening and triggering the bug: dragging instructions in the events editor triggers change in a the instruction list of this instruction. If you try to drop the instruction in a list that changed boooom everything explodes.
Makes sense? A bit hard to get surely at first. Put it another way, here is the current problem:
You have a list of instructions and each instructions have a list of (sub)instructions:
Instruction1
Instruction2
-Subinstruction1
-Subinstruction2
-Subinstruction3
Instruction3
When you want to drag for example Instruction1 and Instruction3 after Subinstruction2, who do you proceed? Knowing that any add/remove operation on a list will (potentially) reallocate it and make all pointers/reference/iterator to it invalid.
from gdevelop.
I think we can create a template class SPtrList and use it for the EventsList and the InstructionsList. The problem is the copy ctor : they are not the same. EventsList uses CloneRememberingOriginalEvent to copy the event and InstructionsList will probably use the copy ctor of each Instruction.
from gdevelop.
See the branch bugfix/instructions-list in my fork.
I've created a template class to manage a vector of shared_ptr. The InstructionsList is just a typedef of it for gd::Instruction. I may also use it for the EventsList but not as a simple typedef as it has some other methods (serialization and recursive search).
from gdevelop.
Taking a look at it. I think that EventsList can be kept as it is now! :)
from gdevelop.
Good job! The code looks great, good use of make_shared
, comments and methods names are consistent with the other GD classes. Really nice what you came up with :)
As I said EventsList can be kept separately, no need to bother with it I think - if we want to refactor it later it won't be much difficult as you class is generic.
Great job, it's better than anything I was dreaming of. 😄
Send me a PR and I'll merge it if you think everything is ok. When done I'll take a look again and this bug to see if I can rewrite the drag'n'drop method. :)
from gdevelop.
I don't see why you want to rewrite the d'n'd method as it's not crashing anymore with a list of shared_ptr.
(for the interface, I've just copied gd::EventsList and renove "Event" from the method's names).
from gdevelop.
Ah oO Cool cool! I thought it could still be crashing because even if the implementation of the lists changed, the semantic and usage of lists is still the same (so it could have lead to the same issues with using deleted instructions).
So that just perfect, thanks a lot! I'm waiting for the PR 😄
from gdevelop.
Related Issues (20)
- Tiled Sprite Randomization Feature (Randomized Rotation and Offset combined with blending) a la Construct 3's r321 update. HOT 2
- Limitation in Physics Engine Speed HOT 1
- Simpler way to center origin sprite point HOT 3
- Add sorting for assets bin HOT 1
- Snapping objects in editor
- Corrupted value in Scene editor's tooltip right after instance deletion HOT 1
- Fullscreen Detection broken HOT 4
- Adding the ability to specify Cordova and NPM plugin parameters in extension dependencies HOT 1
- Platformer Deacceleration shows the wrong tooltip HOT 1
- High-Speed Collision Detection Limitation HOT 3
- Crash while using an editor
- Toggle switches don't work in 5.3.196 HOT 2
- User Interface Sound Effects
- "Preload as" toggles for audio files in the Resources Panel do not behave the same as the "Preload as" event actions. HOT 1
- Crash while using an editor
- Crash while using an editor
- Electron 18.x.y has reached end-of-support
- Text related actions and conditions suggest objects that are not concerned
- [5-5.3.200 bug] (PLATFORMER OBJECT) is on platform (PLATFORM OBJECT) condition no longer works HOT 1
- Top-down Tanks Redux - Kenney HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from gdevelop.