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

[Feature]: Reduce duplicative code around craft recepies #118

Open
1 task
Lazerbeak12345 opened this issue Oct 26, 2023 · 3 comments
Open
1 task

[Feature]: Reduce duplicative code around craft recepies #118

Lazerbeak12345 opened this issue Oct 26, 2023 · 3 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Lazerbeak12345
Copy link

Lazerbeak12345 commented Oct 26, 2023

In minetest-whynot/whynot-game#170 (comment)

It seems all code with @section craft has a lot of duplicative behavior shared with (virtually) all armor. The license isn't compatible, but I've seen that Instant Ores does this in a better way, by making a function for this. Edge cases would be best served the way this code is currently written, with abstractions for the most common cases.

(This issue is not blocking that pull request).

A few arguments for this:

  1. Less code is less bugs
  2. Less code is less work to maintain
  3. Less code is less code to update when changes happen.
  4. A lot of this code is actually documentation. It's nearly (if not exactly the same) across all of them, and could be unified to one place.
  5. Edge cases can still be done alongside the conventional function call.
  6. A lot of different armor mods outside of this modpack could make great use of this new API.

Problem:

I don't know where the function would go, or what it would and wouldn't cover.

Requirements

@BuckarooBanzay
Copy link
Member

Proposals/PR's welcome 👍

@BuckarooBanzay BuckarooBanzay added enhancement New feature or request documentation Improvements or additions to documentation labels Oct 27, 2023
@Lazerbeak12345
Copy link
Author

Lazerbeak12345 commented Oct 27, 2023

Early draft API proposal:

A few new functions, place on the armor table and exact names are TBD

  • A function to register each craft recipe for each armor material. (EX: armor.register_chestplate_recepie("diamond", armor.materials.diamond))
  • A function to call each of the above (EX: armor.register_standard_recepies("diamond", armor.materials.diamond)

For exceptions like the two wooden shields, using only the abstractions above would be a bad idea. They should use the current API for recipe exceptions.

The registration function for shield crafting should be owned by the shields mod. The sheilds mod could perhaps override the "standard recipes" function to also register a shield. Alternatively, a data-oriented approach could be taken, and the "standard recipes" function could determine what functions to call via a list of function references stored in an accessible place.

Recipe names will be the most difficult part if the API is to work for 3rd party armor mods.

@BuckarooBanzay
Copy link
Member

It seems all code with @section craft has a lot of duplicative behavior shared with (virtually) all armor.

uh, before i forget: make sure to don't mess up the docs if possible: https://minetest-mods.github.io/3d_armor/reference/topics/helmets.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants