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

Radiation API #651

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Radiation API #651

wants to merge 19 commits into from

Conversation

DustyDave961
Copy link

@DustyDave961 DustyDave961 commented Sep 28, 2024

Radiation resistance API made with the help of Microsoft Copilot to fix #605.

Api added with the help of Copilot AI.
"rad_resistance" group added to lead block node definitions. Playtesting required.
Lead block removed from node_resistances table.
@DustyDave961 DustyDave961 marked this pull request as ready for review September 28, 2024 18:53
Copy link
Contributor

@DustyBagel DustyBagel left a comment

Choose a reason for hiding this comment

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

Seems fine. This should probably be reviewed by someone who understands how this works better though.

technic/radiation.lua Outdated Show resolved Hide resolved
Greater than or equal to 0 because negative makes no sense.
technic/radiation.lua Outdated Show resolved Hide resolved
technic/radiation.lua Show resolved Hide resolved
technic/radiation.lua Outdated Show resolved Hide resolved
technic/radiation.lua Outdated Show resolved Hide resolved
technic_worldgen/nodes.lua Show resolved Hide resolved
The technic.register_rad_resistance function added
Unnecessary function removed.
Spaces bothered me.
The node_radiation_resistance function is only used locally.
Documented rad_resistance group.
I guess we only need the rad_resistance group?
Space caused an error for some reason?
Documented technic.register_group_resistance function
Added rad_resistance group to resistant nodes.
Technic specific node resistances moved to node definitions.
Comment on lines +183 to +186
technic.register_group_resistance("concrete", 16)
technic.register_group_resistance("tree", 3.4)
technic.register_group_resistance("uranium_block", 500)
technic.register_group_resistance("wood", 1.7)
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately this only works on nodes that are already registered when technic is loaded.
A few ideas to fix that:

  1. re-add the previous cache_radiation_resistance to lazily calculate the node value when needed
  2. calculate all values either in core.register_on_mods_loaded or in a core.after(0, function() ??? end) callback (first server step)

Copy link
Author

@DustyDave961 DustyDave961 Oct 20, 2024

Choose a reason for hiding this comment

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

I found such functions in the Minetest lua api, but I'm unsure how to use the minetest.register_on_mods_loaded(function()) function. Do I just replace "function" with node_radiation_resistance, or is there more to it? I'm still quite new to all this.

Edit: @DustyBagel has informed me that function is not backwards compatible with certain versions of Minetest. I don't think it's a big deal, but should we consider the minetest.after function instead?

Copy link
Member

@SmallJoker SmallJoker Oct 20, 2024

Choose a reason for hiding this comment

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

core.register_on_mods_loaded (core == minetest) is run after all mods are loaded. You can still use core.override_item if needed.

Here's what I would do: loop through core.registered_nodes and cache the resistance values (where it is present). Afterwards, node_radiation_resistance() could always retrieve the value (or 0) from the cache without any additional computations. That's computationally more expensive on startup, but has the lowest memory footprint in the long-run and more predictable code execution time.


Changelog 0.4.16 → 5.0.0 mentions this callback. Minetest 5.0.0 is already the minimal requirement for technic, thus this is not of a concern.

If functions are not mentioned in the changelog, you might as well have a look at the git blame of the lua_api.md line, which reveals when it was added.

technic/radiation.lua Show resolved Hide resolved
technic_worldgen/nodes.lua Show resolved Hide resolved
Why is whitespace such an issue? There's nothing there.
I thought I removed the white space already...
I think this will work?
Radiation functions moved to Radiation section.
Shouldn't the resistance cache be initialized before the node_radiation_resistance function?
@SmallJoker SmallJoker self-requested a review November 20, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to declare nodes as radiation protection and corium-resistant
3 participants