ddnet
Add option for parallax-aware zoom
How this works
Assume that zoom is moving the camera closer/further away and scale layers consistent with parallax parameters.
Use cl_parallax_zoom 1 to enable this behavior.
Screenshots
Video: https://youtu.be/W7eXQN0gRFI.
Springlobe 3
With the settings turned on:
Without:
Beyond Dreams
With new setting:
Without:
Known issues
- Might not look good on all maps. Might break some map effects, although
maximum(parallax{x,y})should be sufficient for the cases where the layer group is supposed to align to game layer in at least one dimension and is set to 100.0f - unless the other one is "in front of" the game layer and is moving even faster across the screen. - Could be inconvenient in the Editor (needs to be switched off to access background Quads as they would otherwise be partially off screen). I have an idea on what we can do there, planning a separate pull request for that.
Checklist
- [X] Tested the change ingame
- [X] Provided screenshots if it is a visual change
- [X] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [X] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [X] Tested the change with ASan+UBSan or valgrind's memcheck (optional)
Should we set this as default? Looks better than the old mode in your screenshots.
Would this keep menus working if you zoom out, like that on Time Shop?
I just tested it, it does not make the situation better.
Some ideas on how it can be enabled.
A: Client setting (this version but with default to enabled)
- Looks good on most maps. Doesn't break anything on maps that I've checked. Doesn't affect default zoom
- Does make some maps slightly less good looking in a certain zoom range. This can be addressed by tweaking parallax{x,y} values
B1: Add an option to set zoom-parallax value in the editor
(next to y-parallax and x-parallax. And if unset default to a value that will make the behavior the same as in option A)
- Gives mappers more control
- Might be used to fix Time Shop (will need to check)
- Requires modifying map format (LayerGroup), unsure how much it will complicate the code/support - biggest consideration
B2: Add option to disable at map level
- Can be skipped for maps where it doesn't work well
C: Wait
The final option is to delay making the decision and instead gather more feedback by either temporarily enabling it as default or letting players enable it manually.
So for B1 we would have different zoom settings per layer? I guess B2 is easiest and we just need a new map config value, and can disable it for the maps where it doesn't work well.
So for B1 we would have different zoom settings per layer?
It's similar to the existing parallax settings in that it configures the perceived distance from the camera and we have these per-layer
I guess B2 is easiest and we just need a new map config value, and can disable it for the maps where it doesn't work well. That's my understanding as well.
With B1 my main concern is how complicated the map-reading/writing code will become.
edit: I'm happy with any option, but obviously want to pick whichever is more-or-less the best one :D
Maybe do B2 and wait for feedback? We can get feedback from mappers if they notice that they want to tweak the zoom-parallax value independently from x/y parallax. And if not then at least the known concern about it harming some maps will already have been addressed.
Given how it makes map rendering better, i'd say to get rid of the option and enforce it in the code. With the mapper setting, it should be enough to satisfy most people.
I still have to check with custom entities background and zoom between 5 and 10 which is what most player uses.
Looks like it's a bit tricky with map settings as well, as we don't have client-side settings that are reset on map change. Should I make it a "server setting" or add a mechanism for "map settings" (or game settings)?
datapoint: Just confirmed that B1 will be enough to make Time Shop work well with zoom (Parallax Zoom would need to be set to 0 for some layer groups).
Hm, looks like adding a new field isn't very difficult, we can also add some placeholder to Teeworlds to make sure we don't have map format conflicts.
Based on your last comment, it depends on layer?
Maybe use a per layer setting then but that would change map format.
Added per-group parameter. What do you think about the new field and the getter method?
I'm also planning to add to the editor:
- A way to preview the effects of this setting in the editor, while allowing access to the edges of the layers when needed - probably with an off/on switch
- An option for Para Zoom parameter to be synced to maximum(para{x,y}) by default so that users unfamiliar with this feature can use the default good value instead of letting it out of sync unintentionally.
The second editor improvement might be important to add before merging this.
Pretty much ready. I need to fix the problem with Quad Env Points scaling (visual issue in the editor when Zoom button is enabled). But it's already usable and nothing seems broken with Zoom button switched off.
Any concerns about the map version increment? That might conflict with Teeworlds doing an increase. Do we not care or should we try to get this change into Teeworlds too?
Maybe something like this? https://github.com/Fireball-Teeworlds/teeworlds/commit/580c01c2abeab094b573fab3bee882dee2e333df
edit: this message originally linked commit in teeworlds/teeworlds repo that must've been automatically created for a pull request draft: https://github.com/teeworlds/teeworlds/commit/580c01c2abeab094b573fab3bee882dee2e333df
(TODO)
- [x] Run newest build with ASan & UBSan. Try to test unusual (<0, >100) Parallax Zoom values and get MouseWZoom ~= 0
- [ ] Record a video with Time Shop fix process where new Editor controls are visible (to get feedback)
Any concerns about the map version increment?
Yes, it should go into our own group object instead.
Yes, it should go into our own group object instead.
Do you mean creating a new MapItem object and storing a reference (datafile index) to that object in the original one? (In order to make it easier to add fields in the future).
One benefit of using the existing object is that if teeworlds at some point implements zoom (either for spectating or as a gameplay change), we could reuse the same field as the field's function is not ddnet-specific.
Do you mean creating a new MapItem object
Yes.
and storing a reference (datafile index) to that object in the original one? (In order to make it easier to add fields in the future).
No, I wouldn't modify the original object in any way. I'd maybe match them by their datafile item IDs (i.e. same ID of group and extended group objects describe the same group).
One benefit of using the existing object is that if teeworlds at some point implements zoom (either for spectating or as a gameplay change), we could reuse the same field as the field's function is not ddnet-specific.
The downside is that if Teeworlds were to use this field in any other way, we're incompatible with no way of reconciling. Also, it seems unlikely that Teeworlds will ever implement zoom.
Updated the description - everything is ready from my side.
@heinrich5991 @def- wdyt?
I think let's get it in and give mappers/users a few weeks to try this out. bors r+
![bors[bot] avatar](https://avatars.githubusercontent.com/in/1847?v=4 "Published by a pub.dev verified publisher"){kind=small}