Add moving buttons for order parameters in functions #1159

Bouh · 2019-07-23T15:59:39Z

This is in link with this card on Trello

I've follow your instruction, i can't compile again the gd.js i got errors and it's bored to search how fix it.
Compile and install the environement for gd.js is a bit hard on Windows10, we have errors everywhere.

4ian · 2019-07-23T19:22:27Z

Almost good :) FREE_ prefix is used to say that this method is in fact a function that is not linked to this class, but a "free function" (a function without a class) taking the class as the first parameter. Which is what you did right in the Wrapper.cpp file but you added an extra parameter to the Bindings.idl file. The only parameters are oldIndex and newIndex (which are unsigned long, which roughly translate to size_t in C++ ) but there is no [Const, Ref] ParameterMetadata parameterMetadata (otherwise this would mean that your swapInVectorParameterMetadata method would need a parameter like const gd::ParameterMetadata& parameterMetadata).

I've fixed this, I'll wait for Travis to compile this. You can actually see the error messages, if any, in the Travis page. For example: https://travis-ci.org/4ian/GDevelop/jobs/562664711 I've seen here there was the error message about the extra parameter.

This being said, would be great to have this compiling on Windows 10, but I have little access to a Windows 10 machine these days. I might get a virtual machine for this.

4ian · 2019-07-23T22:16:15Z

Alright this is passing now! This should work but a question though: if we change the order of the parameters, this won't update any existing action/condition/expression using this function. Should we worry about this? Or is it ok not to update anything? What do you think?

Bouh · 2019-07-24T00:42:38Z

if we change the order of the parameters, this won't update any existing action/condition/expression using this function. Should we worry about this? Or is it ok not to update anything? What do you think?

The name of parameters still, i guess the action/condition/expression need name and not the index of parameter. So if just the name is require all should work without refresh action/condition/expression.

This being said, would be great to have this compiling on Windows 10, but I have little access to a Windows 10 machine these days. I might get a virtual machine for this.

Or linux, i guess Windows10 in VM is heavy and we need windows key.

4ian · 2019-07-24T20:54:13Z

The name of parameters still, i guess the action/condition/expression need name and not the index of parameter. So if just the name is require all should work without refresh action/condition/expression.

No actions/conditions depends on the order of the parameters :/ (otherwise, what would happen if you rename the parameter), like when you declares an extension.
So we need to warn the user or automatically update all the calls to this action/condition.

4ian · 2019-07-26T15:42:09Z

I'm ok for merging this as it does not looks like a big problem, but I was thinking maybe we can actually add a warning using a <Snackbar> (https://v0.material-ui.com/#/components/snackbar)? A quick warning "Remember to update any usage of this function in your events" when a parameter is moved :)

Bouh · 2019-07-26T16:18:17Z

Sorry I didn't see the post.
I can't compile gd.js so I couldn't test.

The order is only visual in the action / condition / expression ?
If so then, i guess we can go without alerting the user, peoples read the titles of the parameters before filling them, users don't rely on the order that was created in the function.
Otherwise between updating the events or display a message with snackbar, update the events is better. Because this kind of detail should not require alerting the user to ask him to do something.

A program that asks the user to do something is a little funny ^^

Bouh · 2019-12-11T00:12:28Z

This feature is at this stage since some updates.
I don't know how finish this, i guess it's require cpp code?

4ian · 2019-12-14T13:13:55Z

Basically yes we would need to add in WholeProjectRefactor (https://github.com/4ian/GDevelop/blob/master/Core/GDCore/IDE/WholeProjectRefactorer.cpp and in the bindings: https://github.com/4ian/GDevelop/blob/master/GDevelop.js/Bindings/Bindings.idl#L1488-L1525) a function to refactor the project after two parameters are swapped in an events function (action, condition or expression).
This function of WholeProjectRefactor would be called when you move a parameter. It would go through the project and, whenever the action/condition is found (in events) or the expression is used, then it update the action/condition parameters order (or for the expression, it rewrites the expression with the parameter swapped).

As a starting point, it should be a bit like RenameEventsFunction:

GDevelop/Core/GDCore/IDE/WholeProjectRefactorer.cpp

Lines 337 to 354 in a6dab9a

    
           void WholeProjectRefactorer::RenameEventsFunction( 
        
               gd::Project& project, 
        
               const gd::EventsFunctionsExtension& eventsFunctionsExtension, 
        
               const gd::String& oldFunctionName, 
        
               const gd::String& newFunctionName) { 
        
             if (!eventsFunctionsExtension.HasEventsFunctionNamed(oldFunctionName)) return; 
        
             const gd::EventsFunction& eventsFunction = 
        
                 eventsFunctionsExtension.GetEventsFunction(oldFunctionName); 
        
             DoRenameEventsFunction( 
        
                 project, 
        
                 eventsFunction, 
        
                 GetEventsFunctionFullType(eventsFunctionsExtension.GetName(), 
        
                                           oldFunctionName), 
        
                 GetEventsFunctionFullType(eventsFunctionsExtension.GetName(), 
        
                                           newFunctionName)); 
        
           }

which is implemented like this:

GDevelop/Core/GDCore/IDE/WholeProjectRefactorer.cpp

Lines 550 to 569 in a6dab9a

    
           void WholeProjectRefactorer::DoRenameEventsFunction( 
        
               gd::Project& project, 
        
               const gd::EventsFunction& eventsFunction, 
        
               const gd::String& oldFullType, 
        
               const gd::String& newFullType) { 
        
             if (eventsFunction.GetFunctionType() == gd::EventsFunction::Action || 
        
                 eventsFunction.GetFunctionType() == gd::EventsFunction::Condition) { 
        
               gd::InstructionsTypeRenamer renamer = 
        
                   gd::InstructionsTypeRenamer(project, oldFullType, newFullType); 
        
               ExposeProjectEvents(project, renamer); 
        
             } else if (eventsFunction.GetFunctionType() == 
        
                            gd::EventsFunction::Expression || 
        
                        eventsFunction.GetFunctionType() == 
        
                            gd::EventsFunction::StringExpression) { 
        
               gd::ExpressionsRenamer renamer = 
        
                   gd::ExpressionsRenamer(project.GetCurrentPlatform()); 
        
               renamer.SetReplacedFreeExpression(oldFullType, newFullType); 
        
               ExposeProjectEvents(project, renamer); 
        
             } 
        
           }

We would need a gd::InstructionParametersSwapper, much like gd::InstructionsTypeRenamer (to swap parameters in action/conditions) and a gd::ExpressionParametersSwapper, much like gd::ExpressionsRenamer (to swap parameters in expressions).

For gd::ExpressionParametersSwapper, the swap of parameters should be done in OnVisitFunctionNode (see gd::ExpressionsRenamer)
For gd::InstructionParametersSwapper, the swap of parameters should be done in DoVisitInstruction (see example)

(you can see the "Visit" indicates the Visitor pattern).

Would be a few hours for me to do I think... but I've little time these days and want to finish https://trello.com/c/hjy2d6O2/383-performance-improvements-for-collisions-raycast-distance-tests-with-large-number-of-objects-or-very-large-levels and work on https://trello.com/c/cLzur8l0/386-add-support-for-declaring-new-effects-in-extensions as effects are important for games and to let contributors create new effects.
I'll try to do this after these two cards!

Bouh · 2019-12-14T16:07:20Z

Yes I also prefer to see the effects and optimization!

Ok let's leave this aside, in any case it doesn't seem possible for me to do the C++ part. It's more than a few hours that I need just for try do to this.

4ian · 2019-12-27T21:44:38Z

Finally took the time to add these functions :)

So you now have gd.WholeProjectRefactorer.moveEventsFunctionParameter and gd.WholeProjectRefactorer.moveBehaviorEventsFunctionParameter.

Use them like gd.WholeProjectRefactorer.renameEventsFunction or gd.WholeProjectRefactorer.renameBehaviorEventsFunction. Parameters are: project, eventsFunctionsExtension, [eventsBasedBehavior for the Behavior one], eventsFunction.getName() (the name of the function for which the parameter is moved, and finally the oldIndex (a number) and the newIndex (number too).

For example:

    gd.WholeProjectRefactorer.renameBehaviorEventsFunction(
      project,
      eventsFunctionsExtension,
      eventsBasedBehavior,
      eventsFunction.getName(),
      2, 
      3
    );

will move the parameter at position 2 to position 3 (knowing that first position is 0) for the given function of the given behavior in the whole project.

@Bouh do you want to give a try at this? In addition to move the parameter like you did, you now need to make two functions that looks like _makeRenameFreeEventsFunction and _makeRenameBehaviorEventsFunction (except that you call them makeMoveXXXParameter, and you pass oldIndex/newIndex and call the proper functions inside) in EventsFunctionsExtensionEditor. Then, pass these functions to <EventsFunctionConfigurationEditor props and finally to <EventsFunctionParametersEditor props.
You'll need to call the proper function (_makeRenameFreeEventsFunction/_makeRenameBehaviorEventsFunction) according to if a behavior is selected or not (see selectedEventsBasedBehavior).

Let me know if you want to try this, otherwise I'll try to do it later.

Bouh · 2019-12-27T22:14:09Z

I will give a try tomorrow. Le ven. 27 déc. 2019 à 22:44, Florian Rival <notifications@github.com> a écrit :

…

Finally took the time to add these functions :) So you now have gd.WholeProjectRefactorer.moveEventsFunctionParameter and gd.WholeProjectRefactorer.moveBehaviorEventsFunctionParameter. Use them like gd.WholeProjectRefactorer.renameEventsFunction or gd.WholeProjectRefactorer.renameBehaviorEventsFunction. Parameters are: project, eventsFunctionsExtension, [eventsBasedBehavior for the Behavior one], eventsFunction.getName() (the name of the function for which the parameter is moved, and finally the oldIndex (a number) and the newIndex (number too). For example: gd.WholeProjectRefactorer.renameBehaviorEventsFunction( project, eventsFunctionsExtension, eventsBasedBehavior, eventsFunction.getName(), 2, 3 ); will move the parameter at position 2 to position 3 (knowing that first position is 0) for the given function of the given behavior in the whole project. @Bouh <https://github.com/Bouh> do you want to give a try at this? In addition to move the parameter like you did, you now need to make two functions that looks like _makeRenameFreeEventsFunction and _makeRenameBehaviorEventsFunction (except that you call them makeMoveXXXParameter, and you pass oldIndex/newIndex and call the proper functions inside) in EventsFunctionsExtensionEditor. Then, pass these functions to <EventsFunctionConfigurationEditor props and finally to <EventsFunctionParametersEditor props. You'll need to call the proper function (_makeRenameFreeEventsFunction/_makeRenameBehaviorEventsFunction) according to if a behavior is selected or not (see selectedEventsBasedBehavior). Let me know if you want to try this, otherwise I'll try to do it later. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1159?email_source=notifications&email_token=AAMX4DTC2GXAFWEL64YFQJTQ2ZZMPA5CNFSM4IGGCUUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHXZXPY#issuecomment-569351103>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMX4DTH62QCJ4SGWC5EFBDQ2ZZMPANCNFSM4IGGCUUA> .

Bouh · 2019-12-28T13:01:03Z

Next on #1340

arthuro555 · 2020-01-26T15:43:52Z

This should be closed now that it's continuing in another PR

4ian · 2020-01-26T16:55:41Z

Right, closing this one!

Bouh and others added 3 commits July 23, 2019 17:52

Add moving buttons order in parameter function

ab09fbf

Fix declaration of FREE_swapInVectorParameterMetadata

478eeb8

Run Prettier

f10b59c

Fix swapInVectorParameterMetadata

90ee0e5

4ian added the ⚙️ Addition required in GDevelop.js/GD Core (C++) Something must be added in GDevelop.js for this PR/bug fix label Jul 28, 2019

Bouh closed this Aug 13, 2019

Bouh deleted the Moving_order_parameters_functions branch August 13, 2019 11:10

Bouh restored the Moving_order_parameters_functions branch August 13, 2019 11:15

Bouh reopened this Aug 13, 2019

Bouh mentioned this pull request Dec 28, 2019

Moving order parameters functions #2 #1340

Merged

4ian closed this Jan 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add moving buttons for order parameters in functions #1159

Add moving buttons for order parameters in functions #1159

Bouh commented Jul 23, 2019

4ian commented Jul 23, 2019

4ian commented Jul 23, 2019

Bouh commented Jul 24, 2019

4ian commented Jul 24, 2019

4ian commented Jul 26, 2019

Bouh commented Jul 26, 2019 •

edited

Bouh commented Dec 11, 2019

4ian commented Dec 14, 2019

Bouh commented Dec 14, 2019

4ian commented Dec 27, 2019

Bouh commented Dec 27, 2019 via email

Bouh commented Dec 28, 2019

arthuro555 commented Jan 26, 2020

4ian commented Jan 26, 2020

Add moving buttons for order parameters in functions #1159

Add moving buttons for order parameters in functions #1159

Conversation

Bouh commented Jul 23, 2019

4ian commented Jul 23, 2019

4ian commented Jul 23, 2019

Bouh commented Jul 24, 2019

4ian commented Jul 24, 2019

4ian commented Jul 26, 2019

Bouh commented Jul 26, 2019 • edited

Bouh commented Dec 11, 2019

4ian commented Dec 14, 2019

Bouh commented Dec 14, 2019

4ian commented Dec 27, 2019

Bouh commented Dec 27, 2019 via email

Bouh commented Dec 28, 2019

arthuro555 commented Jan 26, 2020

4ian commented Jan 26, 2020

Bouh commented Jul 26, 2019 •

edited