Time Nick Message 12:12 Desour sorry for merging a regression, due to not testing. will push trivial fix https://github.com/Desour/minetest/commit/4fb28a8f2f2b50d6a26d0b83ae4fbf436611e78a in 15 min unless there are objections 12:14 Desour actually, devtest has also allfaces_optional nodes that use visual_scale 12:15 Desour not too sure about them. should they support it? 12:18 Desour => should probably just revert until this is figures out 12:30 sfan5 i would just include allfaces_optional 12:31 Desour before PR, allfaces optional are scaled depending on leaves style setting 12:31 Desour it might actually be helpful to not allow it, so modders don't depend on the behaviour 12:32 Desour also, in case we'd in the future add support for visual_scale for glasslike and solid nodes (then the behaviour of such nodes would change) 12:33 Desour on the other hand, for not breaking mods, including allfaces_optional is nicer 12:43 Desour support for allfaces_optional would need updating the spec (aka doc): https://github.com/Desour/minetest/commit/defea26317cc7e5f57f19c5736e193b5ea21d101 12:43 Desour and it's broken if held in hand (the issue that the PR fixed) 12:43 Desour ==> not simple enough to go into a simple pushed commit imo 12:47 Desour as a fix for the regression is not trivial )) 12:48 Desour as a fix for the regression is not trivial (requires decisions), I will push a revert commit in 20 min 13:01 MTDiscord why not push the fix, push another commit when allfaces_optional is decided later? 13:02 Desour_ because then we'd have a regression for allfaces_optional 13:03 Desour_ and just pushing the fix would be the decision to not support visual_scale for allfaces_optional, which would likely lead to nobody doing a PR. I don't want it to be decided that way 13:04 MTDiscord i think not supporting allfaces_optional is reasonable, the docs currently do not include it 13:05 MTDiscord but feel free to just revert if you think this needs more discussion 13:05 Desour_ I think I aggree. but do we not want this to be decided in a PR, so other coredevs and users have time to bring up objections? 13:07 MTDiscord what even is allfaces_optional? 13:08 Desour_ @the4spaceconstants2181: the doc doesn't mention the setting name explicitly, but it's the drawtype you control with leaves_style 13:09 Desour_ the 20 min are over, will push a revert now 13:11 sfan5 I think we should preserve the current scale behavior for drawtypes where it works 13:12 sfan5 so that nodes look the same in the inventory and the world 13:18 Desour_ allfaces_optional looks small in hand if you set leaves_style to simple (#17227) 13:18 ShadowBot https://github.com/luanti-org/luanti/issues/17227 -- Possible regression with `visual_scale` rendering 13:19 sfan5 maybe we should just deprecate/remove it then 13:19 sfan5 (for allfaces_optional that is) 13:21 Desour_ it was never officially supported 13:22 Desour_ well, unless you modders assume that allfaces being supported implicitly means that allfaces_optional is supported if the setting is the right way 13:22 Desour_ s/you// 15:07 MTDiscord Opened a new pr: #17249 15:07 ShadowBot https://github.com/luanti-org/luanti/issues/17249 -- Disable Visual Scale For Unsupported Drawtypes: Attempt 2. by DragonWrangler1 15:09 MTDiscord I haven't added support for the allfaces_optional drawtype just yet.. Figured I'd wait until there is more of a decision on that.. The main regression happened because my pr used "OR" logic, not "AND", which I have fixed. plantlike no longer gives the warning.. I tested with the petz items that have unsupported drawtypes, those work as intended.. (or appear to)