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

Rewrite recipe code and add group support #338

Merged
merged 8 commits into from
Jan 14, 2024
Merged

Conversation

OgelGames
Copy link
Contributor

Rewrite of recipe code to fix some issues and add new features. (sorry this took so long)

  • Recipes now support groups (for input items)
  • All recipes used by technic machines (including cooking recipes) are now cached for performance.
  • Recipes are now processed and cached immediately after mod loading.

Fixes #201, and the issues discussed in #328 and #335.

@nonfreegithub @ThePython10110

@OgelGames OgelGames added Enhancement New feature or request Performance Aims to improve technical performance. Bug fix Testing needed Requires extended testing before working on issue, closing issue or merging pull request. Compatibility Aims to improve compatibility with something. labels Jan 8, 2024
@OgelGames
Copy link
Contributor Author

Hmm, something seems to have broken mineunit 👀

Failure → spec/lv_network_spec.lua @ 122
LV machine network smelts ores
Failure → spec/lv_network_spec.lua @ 132
LV machine network grinds ores
Failure → spec/lv_network_spec.lua @ 142
LV machine network comperess sand

I tested these in-game and they work as normal.

Copy link
Member

@BuckarooBanzay BuckarooBanzay left a comment

Choose a reason for hiding this comment

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

didn't test but code looks sane enough (not sure about the mineunit tests though :/ )

@S-S-X
Copy link
Member

S-S-X commented Jan 9, 2024

Basically Mineunit isn't approving this:

table.insert(minetest.registered_on_mods_loaded, 1, cache_all_recipes)

This was one of those things where I wanted to add clear errors and requirement for explicit configuration to allow questionable things. Currently results are still mixed: some stuff has clear asserts, some is fully protected, some logs warnings and some is permissive.

Fixtures can be used and in other projects have been used to patch similar things but personally I'd recommend against it.

In this case, instead of silently adding required features through fixtures, I'd vote for some public shaming and pointing the finger at Mineunit until fixed.

Copy link

Mineunit failed regression tests, click for details

Regression test log for Technic CNC:


Regression test log for Technic Chests:


Regression test log for Technic:

++ Executing suite spec/api_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin
◌◌◌
++ Executing suite spec/building_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin

++ Executing suite spec/hv_network_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin
◼◼
++ Executing suite spec/lv_network_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin
◼◼◼◼
++ Executing suite spec/network_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin

++ Executing suite spec/nodes_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin

++ Executing suite spec/supply_converter_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin
◌
++ Executing suite spec/tools_compatibility_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin
⭆ Starting test set tools_compatibility_spec.lua:11 ⯈ Technic power tool compatibility
W:	Deprecated technic.register_power_tool use. Setting max_charge for oldlegacy:powertool
W:	Deprecated technic.register_power_tool use. Setting max_charge for oldminimal:powertool
W:	Deprecated technic.register_power_tool use. Setting max_charge for oldhalfway:powertool
W:	Deprecated technic.register_power_tool use. Ensuring fields for oldlegacy:powertool
W:	Using metadata charge values for oldlegacy:powertool
W:	Updated legacy Technic power tool definition for oldlegacy:powertool
W:	Deprecated technic.register_power_tool use. Ensuring fields for oldminimal:powertool
W:	Using metadata charge values for oldminimal:powertool
W:	Updated legacy Technic power tool definition for oldminimal:powertool
W:	Deprecated technic.register_power_tool use. Ensuring fields for oldhalfway:powertool
W:	Mod oldhalfway seems to be aware of technic.plus but oldhalfway:powertool is still using deprecated registration, skipping meta charge compatibility.
W:	Updated legacy Technic power tool definition for oldhalfway:powertool
🢆 Running tests from tools_compatibility_spec.lua:92 ▷ Technic power tool compatibility oldlegacy:powertool can be used (minimum charge)
W:	Use of deprecated function technic.set_RE_wear with stack: oldlegacy:powertool
🢆 Running tests from tools_compatibility_spec.lua:106 ▷ Technic power tool compatibility oldlegacy:powertool can be used (minimum charge + 1)
W:	Use of deprecated function technic.set_RE_wear with stack: oldlegacy:powertool
🢆 Running tests from tools_compatibility_spec.lua:92 ▷ Technic power tool compatibility oldminimal:powertool can be used (minimum charge)
W:	Use of deprecated function technic.set_RE_wear with stack: oldminimal:powertool
🢆 Running tests from tools_compatibility_spec.lua:106 ▷ Technic power tool compatibility oldminimal:powertool can be used (minimum charge + 1)
W:	Use of deprecated function technic.set_RE_wear with stack: oldminimal:powertool

++ Executing suite spec/tools_spec.lua
E:	Loading common/fs from 5.4.1 failed, trying to use builtin


213 successes / 6 failures / 0 errors / 4 pending : 46.233134 seconds

Pending → spec/api_spec.lua @ 133
Technic API Machine registration registers my_mod:my_battery
spec/api_spec.lua:133: Battery box registration does not include all fields

Pending → spec/api_spec.lua @ 190
Technic API Machine registration registers my_mod:machine_base
spec/api_spec.lua:190: Base machine registration does not include all fields

Pending → spec/api_spec.lua @ 285
Technic API internals technic.cables TBD, misleading name and should be updated
spec/api_spec.lua:285: TBD technic.cables naming and need, see technic networks data for possible options

Pending → spec/supply_converter_spec.lua @ 78
Supply converter building overloads network
spec/supply_converter_spec.lua:78: overload does not work with supply converter

Failure → spec/hv_network_spec.lua @ 96
HV machine network smelts ores
spec/hv_network_spec.lua:102: Expected (number) 0 to be more than (number) 10

Failure → spec/hv_network_spec.lua @ 106
HV machine network grinds ores
spec/hv_network_spec.lua:112: Expected (number) 0 to be more than (number) 10

Failure → spec/lv_network_spec.lua @ 122
LV machine network smelts ores
spec/lv_network_spec.lua:128: Expected (number) 0 to be more than (number) 10

Failure → spec/lv_network_spec.lua @ 132
LV machine network grinds ores
spec/lv_network_spec.lua:138: Expected (number) 0 to be more than (number) 10

Failure → spec/lv_network_spec.lua @ 142
LV machine network comperess sand
spec/lv_network_spec.lua:148: Expected (number) 0 to be more than (number) 10

Failure → spec/lv_network_spec.lua @ 152
LV machine network cuts power when generators disappear
spec/lv_network_spec.lua:177: Expected (number) 36700 to be less than (number) 20000

@OgelGames
Copy link
Contributor Author

Tested this again, everything seems to be working correctly, so I think this is safe to merge.

@OgelGames OgelGames added Bug fix and removed Testing needed Requires extended testing before working on issue, closing issue or merging pull request. Bug fix labels Jan 14, 2024
@OgelGames OgelGames merged commit a280a14 into master Jan 14, 2024
8 of 10 checks passed
@OgelGames OgelGames deleted the recipes_rewrite branch January 14, 2024 04:03
@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
Bug fix Compatibility Aims to improve compatibility with something. Enhancement New feature or request Performance Aims to improve technical performance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Machines don't support item groups
4 participants