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

Mods update with new script #116

Merged
merged 32 commits into from
Aug 7, 2022

Conversation

dacmot
Copy link
Collaborator

@dacmot dacmot commented Jul 22, 2022

No description provided.

@dacmot dacmot self-assigned this Jul 22, 2022
mods/decor/home_vending_machines/crafts.lua Show resolved Hide resolved
mods/decor/homedecor_lighting/init.lua Outdated Show resolved Hide resolved
mods/decor/homedecor_seating/armchairs.lua Outdated Show resolved Hide resolved
mods/decor/homedecor_tables/misc.lua Outdated Show resolved Hide resolved
mods/decor/homedecor_wardrobe/init.lua Outdated Show resolved Hide resolved
mods/tools/boost_cart/init.lua Show resolved Hide resolved
mods/tools/hopper/nodes/hoppers.lua Show resolved Hide resolved
mods/tools/hopper/nodes/sorter.lua Show resolved Hide resolved
@dacmot
Copy link
Collaborator Author

dacmot commented Jul 31, 2022

Indeed, the branch https://github.com/mt-mods/homedecor_modpack/tree/stair_complaining has no PR and mt-mods/homedecor_modpack#17 has essentially had no discussion since January.

I'm not very familiar with the whole stairs issue, and so I ask for advice in complete ignorance: what from the new changes breaks anything vs the latest release 2022-06-23? Is the latest release broken? Could this be fixed by freezing homedecor on an older commit?

@wsor4035
Copy link

wsor4035 commented Jul 31, 2022

please do your own fully informed research and know your own codebase https://github.com/minetest-whynot/whynot-game/blob/main/mods/libs/whynot_compat/init.lua#L99-L117

pulled from git blame.
commit: 27c324a
pr: #63

the irony here
#63 (comment)

@wsor4035
Copy link

@Lazerbeak12345
Copy link
Collaborator

please do your own fully informed research and know your own codebase https://github.com/minetest-whynot/whynot-game/blob/main/mods/libs/whynot_compat/init.lua#L99-L117

I'm aware of the whynot_compat workaround, but you might not be aware - the purpose of the mod is to provide temporary compatibility fixes - we fully intend to purge that mod.

@Lazerbeak12345
Copy link
Collaborator

Has this been playtested yet?

Copy link
Collaborator

@Lazerbeak12345 Lazerbeak12345 left a comment

Choose a reason for hiding this comment

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

This isn't breaking stairs: #90 (comment) I had misunderstood and thought we had resolved that issue already.

@Lazerbeak12345 Lazerbeak12345 dismissed their stale review August 3, 2022 03:46

Made on bad info.... see last review comment

@dacmot
Copy link
Collaborator Author

dacmot commented Aug 3, 2022

Has this been playtested yet?

Yes. Seems to work fine for me but I didn't try everything. Tried some of the QA block functions, though it outputs to the console and not the log file so it's hard to compare with the previous build. Default check has no issue. There's a few broken, duplicate recipes, and un-obtainable items. It would be nice to have some sort of automated tests using the qa block functions to compare between releases.

@Lazerbeak12345
Copy link
Collaborator

There's a few broken, duplicate recipes, and un-obtainable items.

Are these items newly broken, duplicate or unobtainable, or were these items that way before this PR?

@dacmot
Copy link
Collaborator Author

dacmot commented Aug 5, 2022

I had to make several modifications to qa_block to be able to compare easily. Here are the differences that I found:

  • New broken recipe
    • Unknown input item: "obsidian_glass"
  • New same recipes
    • same recipe homedecor:desk homedecor:desk_locked
    • same recipe homedecor:desk homedecor:desk_locked
    • same recipe homedecor:filing_cabinet homedecor:filing_cabinet_locked
  • Many, many, many new no-sound items. All in decor mods. I think it's new sound api probably has something to do with it. Either it's utterly broken, or qa_block isn't looking in the right place.
    • I'm pretty sure the stairs whynot_compat code fix I made doesn't pass the sounds properly, because instead of (all) sounds missing, it spells out (dug, dig, footstep, place). The no-sound check puts (all) when there's no .sounds attributes, which the compat code adds. But it the new _sounds_def isn't structured the same way so I think maybe the sounds don't get copied. I'll have to dig further into this.

@bell07
Copy link
Collaborator

bell07 commented Aug 5, 2022

Is this PR for trivial updates, like previously #88? To not block trivial updates my proposal is to create own new issue for homedecor update because of not trivial and set the last known-working commit into lib-config-whynot.sh BRANCHES. Then all trivial updates could be merged with this PR and homedecor could be handled with the other issue.

@Lazerbeak12345
Copy link
Collaborator

I agree with @bell07 here - let's separate out the homedecor things into their own issue. Has an issue been made upstream about these breakages?

@dacmot
Copy link
Collaborator Author

dacmot commented Aug 5, 2022

No. I think we're missing a mod called sound_api. Homedecor modpack has an optional dependency on it saying it's needed for sounds. A couple of mods include sound_core_api as submodules, but never really set the .sounds attribute. I want to see about adding it. I will make that a separate PR and revert the homedecor for this one.

@wsor4035
Copy link

wsor4035 commented Aug 6, 2022

No. I think we're missing a mod called sound_api. Homedecor modpack has an optional dependency on it saying it's needed for sounds. A couple of mods include sound_core_api as submodules, but never really set the .sounds attribute. I want to see about adding it. I will make that a separate PR and revert the homedecor for this one.

basic materials uses it embeded, while homedecor uses it as a dep. see https://github.com/mt-mods/sound_api. https://github.com/mt-mods/sound_api/blob/master/DEV.md should help explain this better

@wsor4035
Copy link

wsor4035 commented Aug 6, 2022

see mt-mods/homedecor_modpack#40, it fixed the obsidian glass bug

@dacmot
Copy link
Collaborator Author

dacmot commented Aug 6, 2022

see mt-mods/homedecor_modpack#40, it fixed the obsidian glass bug

Thanks. We will fully update homedecor once we have a build without the game-agnostic changes that were made and broke sounds.

Problem is at the moment notabug.org (all of TenPlus1's mods) doesn't work because the certificate expired... not sure why. Usually a cron job takes care of renewing Let's Encrypt certificates.

Anyway, now I have to revert by hand 😫

@wsor4035
Copy link

wsor4035 commented Aug 6, 2022

Thanks. We will fully update homedecor once we have a build without the game-agnostic changes that were made and broke sounds.

best of luck on maintaining your own fork then

@wsor4035
Copy link

wsor4035 commented Aug 6, 2022

Problem is at the moment notabug.org (all of TenPlus1's mods) doesn't work because the certificate expired... not sure why. Usually a cron job takes care of renewing Let's Encrypt certificates.

https://github.com/minetest-mirrors might help you out with this

@dacmot
Copy link
Collaborator Author

dacmot commented Aug 6, 2022

best of luck on maintaining your own fork then

I think you misunderstood. We're not forking anything. We're just temporarily rolling back homedecor_modpack and basic_materials to older commits to have a working Why Not? game. Then "we will fully update homedecor" for the next release.

@wsor4035
Copy link

wsor4035 commented Aug 6, 2022

I think you misunderstood. We're not forking anything. We're just temporarily rolling back homedecor_modpack and basic_materials to older commits to have a working Why Not? game. Then "we will fully update homedecor" for the next release.

I did misunderstand, thanks for clarifying. feel free to reach out with your findings.

@dacmot dacmot merged commit 74284b9 into minetest-whynot:main Aug 7, 2022
@dacmot dacmot deleted the mods_update_with_new_script branch August 7, 2022 17:21
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.

4 participants