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

Set is_ground_content in node definitions #348

Merged
merged 35 commits into from
Feb 26, 2024

Conversation

SwissalpS
Copy link
Contributor

see pandorabox-io/pandorabox.io#836

some blocks like stainless steel are explicitly set to true, so I didn't touch them although I feel they should be set to false.

@BuckarooBanzay
Copy link
Member

BuckarooBanzay commented Feb 26, 2024

There's a luacheck error:

    technic/machines/LV/solar_panel.lua:52:2: value assigned to field is_ground_content is overwritten on line 61 before use

Also: you can ignore the other test-error(s) they should be fixed with #349

Correct me if i'm wrong but the is_ground_content flag is only relevant at mapgen and determines if the dungeon-gen replaces the nodes so this might be overkill for most artificially placed nodes and items (right?)

EDIT nevermind, didn't read up on pandorabox-io/pandorabox.io#836 but still: items should not have that flag IMO

@BuckarooBanzay BuckarooBanzay added the Enhancement New feature or request label Feb 26, 2024
@SwissalpS
Copy link
Contributor Author

thanks for catching the solar panel error.

@OgelGames
Copy link
Contributor

some blocks like stainless steel are explicitly set to true, so I didn't touch them although I feel they should be set to false.

Yeah, even if those blocks were to be placed by some kind of mapgen, they're not part of the ground.

but still: items should not have that flag IMO

The only change in items.lua is for technic:machine_casing, which is a node.

@OgelGames OgelGames changed the title [squash me] is_ground_content revision Set is_ground_content in node definitions Feb 26, 2024
@SwissalpS
Copy link
Contributor Author

That leaves marble and granite open for discussion. Also forcefields, which I consider not necessary.

@OgelGames
Copy link
Contributor

Marble and granite are found in the ground, so they can be kept as-is. Forcefields are probably not necessary, but it also won't hurt anything to add that one little value :)

@SwissalpS
Copy link
Contributor Author

OK, added removed forcefields from ground content

@BuckarooBanzay
Copy link
Member

The only change in items.lua is for technic:machine_casing, which is a node.

my bad, i just read the diff-header which contained technic:carbon_cloth i should've read 2 lines more to get to the proper definition line :P

@OgelGames OgelGames merged commit 05079d8 into mt-mods:master Feb 26, 2024
5 checks passed
@SwissalpS SwissalpS deleted the is_ground_content branch February 26, 2024 14:18
@Athozus Athozus added this to the 2.0.0 milestone Apr 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants