Feedback Refactoring

[11clock]

Something I highly recommend is the ability to refactor code. This means that you can easily rename any script or variable without having to rename it on every call to it throughout your entire project.

How it would work is that you highlight the script name or variable, right click, and hit "refactor." Then you type the new name and hit "enter" and boom, the whole project now refers to the new name.

Obviously it shouldn't be a project-wide impact for local variables and the like (in these cases it would do an object-wide refactor instead).

 

[csanyk]

Such features are available in other IDEs, it's only natural to want them in GMS2. I expect that YYG would want to deliver such a feature eventually, even if it's not their highest priority until they get all of the major features working.

 

[csanyk]

Isn't that called 'Find and replace all'?

No, not exactly. A string token can have different meanings in different contexts.

 

[GMWolf]

No, not exactly. A string token can have different meanings in different contexts.

But given the way GML works... hard to do proper anaylis.
if i do foo.bar = 0; there is no telling it that is the bar variable in obj_a or in obj_b....

 

[Lotias]

 

[11clock]

That doesn't work perfectly. Say you have multiple objects that have a local variable of the same name, and you want to change one but not the other. This isn't ideal for such a situation.

 

[RichHopefulComposer]

Find and replace isn't the same thing as refactoring, guys. That's why they have different names. I love how whenever someone makes a good suggestion for GM, they get shot down with crappy workarounds. It's like half the people on the GMC don't want to see GM improve for some reason.

 

[Mick]

The rename feature in Visual Studio is a refactoring feature: https://msdn.microsoft.com/en-us/library/6kxxabwd.aspx.

I love that feature in VS and I agree with @11clock that it would be nice to have in GMS.

 

[csanyk]

I love how whenever someone makes a good suggestion for GM, they get shot down with crappy workarounds. It's like half the people on the GMC don't want to see GM improve for some reason.

I cannot agree more! We should be talking about how to make GMS better, not how improvements aren't an absolute requirement since there's some inferior workaround already possible. That's the old way of thinking, back when YYG were struggling to maintain the old Delphi codebase. We're in a new era now. GMS2 "called a do-over" on the codebase, and this is the opportunity to get things right.

Back on the old GMC forums, I often posted ideas on the old Suggestion Box forum, asking for various improvements, and kept getting shut down by people saying that it was possible to do what I wanted already, albeit by some clunky workaround that was markedly inferior to the improvement I had suggested. So frustrating!

(But a lot of that was because YYG used Suggestion Box only for suggestions that they could implement in 1.x, and were not taking ideas for GM:Next there. Now that we have the 2.0 beta, it seems they are listening more, and that's a good thing!)

Refactoring is the modification of code to improve stability or performance.

You're right, of course. But the way much refactoring is accomplished is by removing duplication from the codebase. Finding the duplication in the first place is much like... well, Find. And replace is usually replacing a repeated line of code with a function call where that repeated line has been moved. Or replacing a literal value with a named constant. So at the coding level, refactoring frequently resembles Find and Replace. But at a conceptual level, you're correct that it's improving code without changing its external behavior.

 

[Juju]

Global find-replace is a big step up from the previous workflow (basically using FNR.exe and hoping for the best). Manually stepping through each occurrence of an identifier isn't too bad.

I'm not sure specifically how VS does its parsing but I'd imagine it's quite advanced. Or perhaps its as simple as replacing everything that's not delimited as a string.

 

[rui.rosario]

It is much easier for an IDE to apply proper refactoring when you don't have a completely dynamic language such as GML (where assignment to a misspelled variable creates a new one, meaning you cannot know programatically know all variables an object will have and all of its usages unless it applies analytics to all of the code). Not having this information means the IDE cannot have any certainty that it is refactoring all occurrences of a variable (scripts would be easier to refactor for instance). So I think that with GML at its current state then Global Find and Replace is one of the best / safest alternatives we have.

 

[altan0]

I think refactoring is a valid idea and should be implements especially when you are dealing with many script calls.

Replace and find is a crude way of doing it but it would be convenient if renaming a script (in the treeview) would auto rename every call function to that script.

 

[GMWolf]

Guys. Simply because of the way GML works, 'proper' refactoring is simply impractical.
Find and replace is the best we can get.

Picture this:
3 objects, A, B and C.
Object A and B both have a 'name' variable declared in the create event.

Object C has the following line of code: var foo = inst.name;

If you refactoring the name variable in object C. Should that refactor it in object A, B or both?

What about if you rename it in object A. Should it change in object C? Even if object C was intended to interact with object B, not A?

As you can see, outside runtime, there is no way of knowing what objects your variables refer to. Therefore, 'proper' refactoring cannot work.

 

[FrostyCat]

Refactoring is a valid idea, but not a currently feasible idea just yet. The only thing worse than not supporting refactoring, is supporting refactoring and doing it wrong. The dynamic typing in the current iteration of GML makes that highly likely, with or without global replacements.

One potential solution is to introduce helper function like this one, that checks types and also semantically identifies its argument:

/**
Instanceof(object, instance)
*/
{
  if (argument1.object_index != argument0) {
    show_error("Assertion failed: " + string(argument1) + " is not an instance of " + object_get_name(argument0));
  }
  return argument1;
}

Using it with Fel666's example:

var foo = (Instanceof(A, inst)).name;
// OR
var foo = (Instanceof(B, inst)).name;

Along with JSDoc tagging of instance variables, this solves Fel666 and rui.rusario's concern for anyone who uses it consistently. Both the type and variables would be fully machine-readable, which paves the way for not just refactoring. Of course there would still be cases where this isn't or can't be used, but now the refactoring tool is in a good position to show these uncertainties on a case-by-case basis.

If built into the runner as a native function, you can get even more flexibility since the compiler is free to optimize it individually. If performance comes first (e.g. YYC, final builds), just make it do nothing and pull the inner argument out. If correctness comes first (e.g. F5/F6), build it into the runner as a runtime check. And the best thing is that there is no new syntax here.

This isn't the cleanest way to do it, and in the long run it would be best to add some basic support of static typing in isolated cases. But if YoYo doesn't do that in the short term, this shows how it can still be possible in the meanwhile.

 

[GMWolf]

Refactoring is a valid idea, but not a currently feasible idea just yet. The only thing worse than not supporting refactoring, is supporting refactoring and doing it wrong. The dynamic typing in the current iteration of GML makes that highly likely, with or without global replacements.

One potential solution is to introduce helper function like this one, that checks types and also semantically identifies its argument:

/**
Instanceof(object, instance)
*/
{
  if (argument1.object_index != argument0) {
    show_error("Assertion failed: " + string(argument1) + " is not an instance of " + object_get_name(argument0));
  }
  return argument1;
}

Using it with Fel666's example:

var foo = (Instanceof(A, inst)).name;
// OR
var foo = (Instanceof(B, inst)).name;

Along with JSDoc tagging of instance variables, this solves Fel666 and rui.rusario's concern for anyone who uses it consistently. Both the type and variables would be fully machine-readable, which paves the way for not just refactoring. Of course there would still be cases where this isn't or can't be used, but now the refactoring tool is in a good position to show these uncertainties on a case-by-case basis.

If built into the runner as a native function, you can get even more flexibility since the compiler is free to optimize it individually. If performance comes first (e.g. YYC, final builds), just make it do nothing and pull the inner argument out. If correctness comes first (e.g. F5/F6), build it into the runner as a runtime check. And the best thing is that there is no new syntax here.

This isn't the cleanest way to do it, and in the long run it would be best to add some basic support of static typing in isolated cases. But if YoYo doesn't do that in the short term, this shows how it can still be possible in the meanwhile.

It will work for anywhere you used Instanceof..
but say part of your code doesnt use it. Then the refactoring process will break the code, and it can be a nightmare to debug if all objects have the correct name but one!

 

[FrostyCat]

It will work for anywhere you used Instanceof..
but say part of your code doesnt use it. Then the refactoring process will break the code, and it can be a nightmare to debug if all objects have the correct name but one!

That's what I said about "showing the uncertainties on a case-by-case basis" means. Do the renaming on all matches that have the tagging, but also present rough matches without the tagging as optional targets.

Even the refactoring tools on other IDEs have moments of uncertainty too, especially when dealing with reflection or languages that allow void pointers or wild generic types. When they are in doubt about how to proceed, they present the problematic cases for the user to call shots on.

The point is not to eliminate all uncertainty, but to minimize it and make them visible. With the helper functions, the range for manual resolution is narrowed down to just the ones that don't use it. Still a huge productivity boost.

 

[csanyk]

Guys. Simply because of the way GML works, 'proper' refactoring is simply impractical.
Find and replace is the best we can get.

Picture this:
3 objects, A, B and C.
Object A and B both have a 'name' variable declared in the create event.

Object C has the following line of code: var foo = inst.name;

If you refactoring the name variable in object C. Should that refactor it in object A, B or both?

What about if you rename it in object A. Should it change in object C? Even if object C was intended to interact with object B, not A?

As you can see, outside runtime, there is no way of knowing what objects your variables refer to. Therefore, 'proper' refactoring cannot work.

They can change how it works!

Refactoring isn't a simple find and replace operation. It's scope-aware, find-replace + auto-generated new code. If you're doing refactoring manually, yes, you're going to use find-replace, but you are also providing the decisions for scope, and writing some new code yourself. These are tasks that can be improved through the use of good refactoring tools.

It's entirely possible to make this in GMS. There's nothing about a GMS project that makes it impossible or impractical. It would be a good feature to have. It's not an essential must-have, but once other features are mature it'd sure be nice to have some refactoring tools that make it less tedious, manual, and error-prone.

 

[GMWolf]

They can change how it works!

Refactoring isn't a simple find and replace operation. It's scope-aware, find-replace + auto-generated new code. If you're doing refactoring manually, yes, you're going to use find-replace, but you are also providing the decisions for scope, and writing some new code yourself. These are tasks that can be improved through the use of good refactoring tools.

It's entirely possible to make this in GMS. There's nothing about a GMS project that makes it impossible or impractical. It would be a good feature to have. It's not an essential must-have, but once other features are mature it'd sure be nice to have some refactoring tools that make it less tedious, manual, and error-prone.

Im not sure you get the point... but eh. whatever.

 

[FrostyCat]

They can change how it works!

Refactoring isn't a simple find and replace operation. It's scope-aware, find-replace + auto-generated new code. If you're doing refactoring manually, yes, you're going to use find-replace, but you are also providing the decisions for scope, and writing some new code yourself. These are tasks that can be improved through the use of good refactoring tools.

It's entirely possible to make this in GMS. There's nothing about a GMS project that makes it impossible or impractical. It would be a good feature to have. It's not an essential must-have, but once other features are mature it'd sure be nice to have some refactoring tools that make it less tedious, manual, and error-prone.

I hope you can hear what you're saying, because scope awareness is the part that we've repeatedly called out as infeasible in GML without further adaptations. And we all have evidence to support our claims.

Let's show Fel666's example one more time:

var foo = inst.property;

Let's say both objects A and B have a variable named property, and I'm now refactoring property in B only to become newproperty. Should the refactoring command touch this piece of code?

The answer is that without further context on inst, this question simply cannot be answered. Even with context, determining the type of inst is equivalent to the Halting Problem due to reachability also being equivalent to it. You either have an algorithm that never stops running, or one that gives the wrong answer on certain inputs, or both.

Before GMS can become scope-aware, it must have a way of reliably resolving these ambiguities, manual type marking is one of them. This is part of the problem you must address along with your suggestion, without creating new problems that are equivalent to solving the Halting Problem.

I think I've pointed this out to you before, but you are so focused on the benefits of your suggestions that you don't see the costs of getting there. You just don't look enough moves ahead to make your suggestions convincing. When people do look ahead a couple of moves and point out issues and knock-on effects, you call them being backwards and resistant to change instead of addressing their observations.

When it comes to "looking before you leap", it's not just where you want to leap to but also where you stand.

 

[Mike]

We can not refactor instance variables (as there is no object definition), but we can refactor local and global variables, as well as resource names. We have this on our list already - especially resource names as these are "unique" IDs and we can track where they are. If someone builds up a string and asks for the ID this will obviously fail, but if your using code, we can refactor that in GML and DnD.

We'll probably start out with resource names.....

 

[11clock]

We can not refactor instance variables (as there is no object definition), but we can refactor local and global variables, as well as resource names. We have this on our list already - especially resource names as these are "unique" IDs and we can track where they are. If someone builds up a string and asks for the ID this will obviously fail, but if your using code, we can refactor that in GML and DnD.

We'll probably start out with resource names.....

Any updates about this feature? Refactoring resource names at least would save a ton of headache.

 

[Jobo]

It is on the roadmap https://help.yoyogames.com/hc/en-us/articles/231719448-RoadMap

 

[sir.pinski]

What I'd like to see shorter term is refactoring of object

Any updates about this feature? Refactoring resource names at least would save a ton of headache.

Yeah, this is what I want. I suspect it would be a lot simpler than full refactoring and it's the main thing I need. Right now if I name an object and discover later that I should have used a different name (perhaps the functionality has changed or...whatever) it's a huge pain to change the name because you have to change all references to that name too (both time-consuming and easy to miss some). At this point I don't bother and just leave it with the wrong name, which isn't a good solution obviously.