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

Document air_equivalent as deprecated #13792

Merged
merged 1 commit into from Oct 2, 2023
Merged

Document air_equivalent as deprecated #13792

merged 1 commit into from Oct 2, 2023

Conversation

sfan5
Copy link
Member

@sfan5 sfan5 commented Sep 11, 2023

No description provided.

@sfan5 sfan5 added @ Documentation Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines labels Sep 11, 2023
Copy link
Contributor

@erlehmann erlehmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is really deprecated, IMO the engine should warn in log every time this is accessed. After all, if you actually remove this property, something truthy may become falsy (nil).

”Unclear meaning” is not exactly correct, see: #13722 (comment)

@sfan5 sfan5 merged commit de0036f into minetest:master Oct 2, 2023
@sfan5 sfan5 deleted the aeq branch October 2, 2023 11:44
@erlehmann
Copy link
Contributor

erlehmann commented Oct 2, 2023

Saying that it has an unclear meaning is still wrong, as discussed on IRC after I stopped a previous PR: #13790 (comment)

@SmallJoker what exactly changed since this was last discussion so that you approved it? What should people use instead of using air_equivalent? Both “is this air or is this ignore” and “does this have an airlike drawtype” are most certainly not the correct replacement. I ask this for the 32 mods on ContentDB which use air_equivalent: https://web.archive.org/web/20230911164208if_/https://content.minetest.net/zipgrep/501df1d6-4355-4fd8-8afb-947ea023aa6e/

@SmallJoker
Copy link
Member

SmallJoker commented Oct 2, 2023

@erlehmann Node groups can be used to reflect such properties. Whether or not a node is air-like also depends on the interpretation. Air tanks might only consider breathable air variations as air-like (e.g. recharging air bottles), whereas machines might only care whether the node is some kind of non-explosive gas.
Thus it is best for mods to determine node groups or node definition checks that make sense for them to ensure the best possible compatibility.

PS: In my opinion it would've made sense to remove this undocumented field entirely. It's the modders fault to rely on undefined behaviour - much like UB in C++.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Documentation One approval ✅ ◻️ Trivial The change is a trivial bug fix, documentation or maintenance change, as per the Git Guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants