Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transform snapping moved to AdvancedSettings #46614

Closed
wants to merge 1 commit into from

Conversation

lawnjelly
Copy link
Member

@lawnjelly lawnjelly commented Mar 3, 2021

Transform snapping is made totally customizable, but is removed from project settings. It can now only be set from script etc using a new GDCLASS, AdvancedSettings.

This PR is one of two options, the other is #46615.

About

As 3.2.4 release is approaching rapidly, I have to admit there are several aspects I'm not happy with concerning the transform_snapping (originally proposed by m6502).

  • Although it offers a lot of potential, it seems to introduce as many problems as it solves.
  • In particular it has the tendency to introduce judder due to aliasing (or sampling error) between the snapped items and the fractional actual positions of items, and of course the position of the camera which is again fractional.
  • This aliasing also occurs with anything off of the grid, such as fractionally scaled parallax layers.
  • There is an interaction with physics, whereby physics vibrations can cause jarring amplified vibrations in the snapped result.
  • The best snapping settings may vary in different games (and especially varies according to window stretch_mode).
  • It is clear that users have an oversimplified and idealistic view of how transform_snapping will work and fix their games.

So at this stage we have 3 choices as I see it:

  1. Fully expose snapping settings as project settings, as in Fix 2d camera frame delay #46569
  2. Remove transform snapping, and perhaps revisit it in 3.2.5
  3. Keep the functionality available, but hide it

So far I've been working along the lines of (1), but feedback so far indicates that the majority of people interested in trying the technique are not aware of how it works, when it should be used, and what the pitfalls are. This all suggests that if we make the functionality readily accessible we will be deluged by support requests for issues that are bogus.

There is, as yet, no facility for advanced project settings in 3.2. Even a UI accessible advanced option can be problematic because users are likely to change these while experimenting, and file bogus issue reports for the inevitable side effects.

So instead I'm quite a fan of adding some hidden settings to the engine, that are pretty much 'don't mess with these unless you know what you are doing'. This would seem to be a good place to put snapping functionality, at least for the time being.

I see having the functions available from script as being a compromise between removing the snapping from 3.2.4 entirely, and exposing something which is clearly problematic for beginners.

Typical use

func _ready():
	var ad = AdvancedSettings.new()
	ad.set_transform_snap_2d(AdvancedSettings.SNAP2D_TYPE_ITEM_PRE, AdvancedSettings.ROUND_MODE_ROUND, AdvancedSettings.ROUND_MODE_ROUND)

The AdvancedSettings class is currently derived from Reference rather than Node, I don't think it needs to be a Node. The settings can be set as a one off at startup of the project. This also has the advantage it is harder to accidentally leave one of them set incorrectly, and they are only active at runtime.

Camera

As an aside, the camera delay fixes are very easy to use and definitely beneficial so I'll be splitting these into a separate PR later this morning (I really shouldn't have combined them in the same PR initially).

Transform snapping is made totally customizable, but is removed from project settings. It can now only be set from script etc using a new GDCLASS, AdvancedSettings.
@lawnjelly
Copy link
Member Author

Closing this as the project setting version is preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants