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

Add mana requirements indicator to the mana pool recipes #52

Conversation

socolio
Copy link

@socolio socolio commented May 29, 2024

Oh... how to compare recipes with one pixel mana requirements... time to (test) fix
image

Tested on one of the latest nightly builds.
Only client-side changes (buts also tested on dedicated server for sabity level stability)

Copy link

@Alastors Alastors left a comment

Choose a reason for hiding this comment

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

This is an intentional major design decision of Botania and violates our magic integrity policy

@Dream-Master
Copy link
Member

I think it can be a nice option to see how much a recipe use instead of go to a wiki / view the code to find out how much it use. Sure we can think of a mechanic to unlock this with some magical research

@Dream-Master Dream-Master requested a review from a team May 29, 2024 20:14
@socolio
Copy link
Author

socolio commented May 29, 2024

I think it can be a nice option to see how much a recipe use instead of go to a wiki / view the code to find out how much it use. Sure we can think of a mechanic to unlock this with some magical research

Current mana bar is accurate enough to meet the needs of the players if mana consumprion is big enought. This change is targeting players which works with low mana recipes.

@Alastors
Copy link

Honestly I'd make the argument that there isn't enough Botania integration to even think about this in that regards 🤷‍♂️

@combusterf
Copy link
Collaborator

I'm with Alastors on this one, this was explicitly designed this way by the original author so I don't feel privileged enough to change that.

@socolio
Copy link
Author

socolio commented May 29, 2024

Honestly I'd make the argument that there isn't enough Botania integration to even think about this in that regards 🤷‍♂️

In GTNH there exist some sets of recipes where mana consumption fluctuates so enought that it is hard to distinguish real values of mana requirements

@socolio
Copy link
Author

socolio commented May 29, 2024

I'm with Alastors on this one, this was explicitly designed this way by the original author so I don't feel privileged enough to change that.

There is literally no difference between 1000 and 700 mana requirements on low level Botania crafts (according to NEI). That is bad.

@Alastors
Copy link

I'm with Alastors on this one, this was explicitly designed this way by the original author so I don't feel privileged enough to change that.

There is literally no difference between 1000 and 700 mana requirements on low level Botania crafts (according to NEI). That is bad.

🤷‍♂️ it's neither bad nor good, it is simply the mechanic.

@Alastors
Copy link

This is an intentional major design decision of Botania and violates our magic integrity policy

Following up on this, just for case history and all of that, our policy is very explicit in its defense toward maintaining the artistic and mechanical design, here are the policies related to that.

https://github.com/GTNewHorizons/GTNH-Dev-Doc/blob/master/developer's%20code%20of%20conduct.md

  1. Regarding textures within GTNH
  • i. Regarding magic textures: within the vision of the modpack, magic is meant to feel, look, and be different from GregTech, any form of GUI update for the sake of (or that would inadvertently bring) consistency with GT GUIs should be rejected.
  • ii. Regarding new textures: try to keep textures consistent with the mod/mod group they're being added to.
  • iii. Regarding tooltips: reduction is fine when needed, but if there is a specific consistency, try to stay within that.
  • iv. Regarding inventory changing: GUIs that shift the colour of the player's internal inventory are allowed as long as its usage is consistent. Examples being: GT's Steam Machines, Magic Bee's Magic Apiary, Railcraft's Coke Oven, etc.
  1. The design focus of Magic
    As mentioned above, magic within GregTech: New Horizons is meant to have a strong, distinct look, feel, and execution to the typical tech mod in the pack. Thusly, it is of upmost importance that both the artistic and mechanical designs of each mod be treated with the upmost respect and taken into consideration in the creation of new magic content."

Likewise, I think the case history reflects negatively against changes like this:
GTNewHorizons/MagicBees#32

Going further, despite all of the changes Vazkii made against what his original promises were, the lack of distinct numbers in botania was one of the few things that remained.

personal note

I do not state this out of malice, nor in attempt to dissuade you from pursuing magic development in general. I am ecstatic to see people passionate about magic in general, and I desperately hope to see you continue this passion in the future. Thank you for your contribution regardless of whether it makes it into the pack or not.

@GedeonGrays
Copy link

Just a reminder, Alfheim (Botania addon) already implements numerical mana representation (config option).
Maybe another reason to add Alfheim to GTNH ¯_(ツ)_/¯


int mana = ((CachedManaPoolRecipe) arecipes.get(recipe)).mana;
String manaRequired = Integer.toString(mana);
FontRenderer font = Minecraft.getMinecraft().fontRenderer;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tab/spaces need to be fixed


GuiDraw.drawTexturedModalRect(45, 5, 38, 35, 92, 50);
HUDHandler.renderManaBar(32, 63, 0x0000FF, 0.75F, mana, TilePool.MAX_MANA / 10);
font.drawString(manaRequired, 84 - font.getStringWidth(manaRequired) / 2, 71, 0xFF03737F);
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be eligible, this needs a config option that defaults to not showing the numerical mana value

@Dream-Master
Copy link
Member

We do not have to adhere to the specifications and standards of the original authors and are free to decide what fits into the GTNH and what does not. Everyone should think freely and break new ground. Thank you.

@chochem
Copy link
Member

chochem commented May 29, 2024

I am with alastor and combusterf as well. This is antithetical to the basic design principles of botania that make people like the mod so much.

@Connor-Colenso
Copy link

I mean, I would be in favour. I get the original author hated it, but I can't really see a good reason to not allow us to peak into the inner workings so to speak. It just feels like "no because someone 10 years ago said so" which isn't really relevant anymore.

@Alastors
Copy link

Alastors commented May 29, 2024

image
Why don't we just port the book zoom feature into nei, then we stay in line with the original intentions and design, then everyone's happy

@dvdmandt
Copy link

I'm torn. The inability to see exact numbers are indeed thematical and the original design and all that, and it's nice and immersive when you're just doing simple stuff by hand imho, but as soon as you want to really use it for something, you'll likely start looking at wikis, source code or spreadsheets to get the actual numbers anyway. I guess I'm not against the idea of adding some form of item or research (botanical engineering - the science of mystery?) or something to unlock this at least.

@Dream-Master
Copy link
Member

lets do it like thaumcraft show the full recipe with values when you researched it.

@socolio
Copy link
Author

socolio commented Jun 5, 2024

PR may be rejected due to better solution #53

@Alastors Alastors closed this Jun 5, 2024
@Alastors
Copy link

Alastors commented Jun 5, 2024

Closed as per request of the PR owner

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.

8 participants