Rename Control margin to offset

[madmiraal]

As originally identified here, Control's margin is actually an offset (relative to the anchor). In addition, margin uses the enum Margin to specify the side. However, the Margin enum is not just used by Control's margin, it is also used by other properties to specify the side. Since these properties are not limited to other margins, for example Control's anchor also uses it, the enum Margin should be renamed Side and its elements SIDE_*.

Therefore, this PR:

• Renames Control's margin to offset
• Renames enum Margin to Side and its elements from MARGIN_* to SIDE_*
• Renames GraphNode's offset to position_offset

Note: GraphNode's offset property needed to be renamed to avoid hiding its parent's new offset property. It was renamed position_offset to reflect its use and to align with GraphEdit's scroll_offset to which it is relative to.

Part of #16863.

[groud]

I did not check everything line by line, but it seems good overall.

[akien-mga]

Thanks!

[aaronfranke]

What about the names of the methods with "margin" in them? For example, I think Rect2's grow_margin should be grow_side.

[madmiraal]

What about the names of the methods with "margin" in them? For example, I think Rect2's grow_margin should be grow_side.

See #44751. Are there any others?

[aaronfranke]

Any of the methods affected in the diffs of this PR are candidates, but I'm not sure which should be changed. I think each would need to be investigated individually. Here's a not-necessarily-comprehensive list:

Camera2D:

• get_drag_margin
• set_drag_margin

NinePatchRect:

• get_patch_margin
• set_patch_margin

StyleBox:

• get_default_margin
• get_margin
• set_default_margin

StyleBoxFlat:

• get_expand_margin
• set_expand_margin

StyleBoxTexture:

• get_expand_margin_size
• get_margin_size
• set_expand_margin_all
• set_expand_margin_individual
• set_expand_margin_size
• set_margin_size

TextureProgressBar:

• get_stretch_margin
• set_stretch_margin

[pouleyKetchoupp]

At the moment, all control positions are broken when opening a previous project on master and it's cumbersome to fix by hand.

What would the best way to convert the scenes automatically when they are opened? Would extra compatibility code in Control::_set make sense to load old margin information?

[madmiraal]

@aaronfranke My assessment:

• Camera2D: Drag margins are the proportion of the distance from the centre to the side that will be used before dragging the camera. It definitely shouldn't be renamed offset, but I'm not convinced margin is the right name either.
• NinePatch: Patch margins are the widths of the row or column next to the side. Again, it definitely shouldn't be renamed offset, but I'm not convinced margin is the right name either.
• StyleBox, StyleBoxFlat and StyleBoxTexture: These are definitely margins.
• TextureProgressBar: These are the same as the NinePatch margins.

In conclusion, I don't think any of the others need changing.

[groud]

What would the best way to convert the scenes automatically when they are opened? Would extra compatibility code in Control::_set make sense to load old margin information?

hmm, I though we add a way to register this kind of rename. But we have it only for classes, not properties if i am not mistaking.

It's likely worth implementing though.

[pouleyKetchoupp]

I've opened #44820 for the issue with backward compatibility. It needs to be addressed for 4.0 release since there is currently no warning when it breaks.

[aaronfranke]

I was using Godot 2.1 for testing and I noticed that in Godot 2.1 margins are actually margins, that is, a larger value for the right and bottom margins means moving left and up instead of right and down, and margins of all 10 lead to a 10px border. I wonder why this was changed in the first place? And why whoever changed the behavior for Godot 3.0 didn't rename the properties?

(This PR is good, this is just a fun fact I noticed).

[groud]

I was using Godot 2.1 for testing and I noticed that in Godot 2.1 margins are actually margins, that is, a larger value for the right and bottom margins means moving left and up instead of right and down, and margins of all 10 lead to a 10px border. I wonder why this was changed in the first place? And why whoever changed the behavior for Godot 3.0 didn't rename the properties?

I did made the change. Inverting the right and bottom margin made sense for consistency reasons. I understand keeping them that way would have made sense if all controls were setup in the "wide" preset, where the margins were effectively acting as margins. But for all other situations (anchored at top left for example), this setup was more confusing. Now, whatever its layout, a control's vertical side with a negative/positive margin (offset) is at the left/right of its anchor, and similarly for horizontal sides.

Not sure for why we did not rename them right away, probably for compatibility reasons, or because we did not ask ourselves the question I suppose.