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)
- In App Purchase is stuck after the purchase HOT 2
- [AdMob] Error occurs after 15-17 times of loading rewarded video
- Standardise Shift and Ctrl key presses across the App. HOT 1
- Crash while using an editor HOT 1
- Mouse Pointer Lock or TimeDelta() bug in Mac browser run games when scenery is moving.
- GDevelop exports corrupted AAB file HOT 2
- Hint is over the JS editor when expanded. HOT 1
- iOS crash "evaulating new THREE_ADDONS.EffectComposer" HOT 3
- eventsFunctionContext.getArgument() doesn't work with "Objects" argument type HOT 2
- An error occured in the Instance editor ... HOT 1
- Typo for 3D stuff HOT 1
- Raycast collision bug,skipping first collision iteration HOT 1
- GDevelop branding should be optional on leaderboards
- Layer effects not working properly on Mobile.
- Improve undeclared variables tip in the variable editor HOT 1
- Crash while using an editor
- tauri
- Using "Take screenshot" action doesn't capture the 3D layers.
- Add better structured variable support to DialogueTree extension HOT 2
- Theatre.js as part of timeline editor for GDevelop HOT 7
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.