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

Enhancements to attack mechanics #25

Merged
merged 8 commits into from
Nov 15, 2024
Merged

Enhancements to attack mechanics #25

merged 8 commits into from
Nov 15, 2024

Conversation

Achille004
Copy link
Collaborator

@Achille004 Achille004 commented Sep 23, 2024

Overview

These changes propose some small but heavily needed tweaks to the weapons, mainly featuring new attack mechanics based on energy levels

Detailed Changes

Core Changes

  • Upgraded NeoForge to version 21.1.55 and Mekanism to 1.21.1-10.7.7.64.
  • Revised the naming of weapons from Meka-- to Meka_-- to align with the Meka-Tool's naming for better consistency.
  • Annotations and imports have been optimized for a clearer codebase.

Attack and Combat Changes

  • Implemented a custom radial version of Mekanism's "Attack Amplification Unit" and revamped the damage and energy evaluation system:

    This new Unit can be simply crafted by converting one from Mekanism in the crafting table, having also the option to do the opposite action.

    The new system increases attack damage based on available energy and the number of enabled attack amplification units.
    The energy bar has been adjusted to change color based on its energy status.

    As example, for High (16):

    • When energy is below the default usage, attack damage is zero.
      This corresponds to a red energy bar.
    • When energy is between default usage and the needed amount, attack damage scales from the base damage (50) to the maximum value (150).
      This corresponds to a yellow energy bar.
    • When energy exceeds the needed amount, attack damage is at its maximum value (150).
      This corresponds to a green energy bar.
  • Made projectiles pick-up-able in survival mode when shot from an actual arrow.

Note: Translations for Korean (ko_kr) and Chinese (zh_cn) are still pending.

Changed weapon naming from Meka* to Meka_* to meek Meka-Tool's naming.
…on Unit" and reworked damage/energy evaluation.

Now attack damage increases with available energy and bonus attack damage (which in turn depends on the enabled mekaweapons:attackamplification_unit(s)).
For example, let's say we have enough units installed to select "High (16)":
  - energy < defaultEnergyUsage --> attackDamage = 0
  - defaultEnergyUsage <= energy < energyNeeded --> baseDamage (=50) <= attackDamage < baseDamage * damageMultiplicator (=150)
  - energyNeeded <= energy --> attackDamage = baseDamage * damageMultiplicator (=150)
Korean(ko_kr) and Chinese(zh_cn) translations are not implemented.
  - green when gun operates normally
  - yellow when energy is not enough for a full shot
  - red when energy is not enough to use
Made projectiles pickup-able in survival when shot from an actual arrow.
@omeranha
Copy link
Owner

hello I highly appreciate your time spent on this pull request, these changes are very importants for the mod value, and sorry for the really late response, i'm gonna test the changes and approve it today

@Achille004
Copy link
Collaborator Author

Achille004 commented Oct 11, 2024

Yeah no worries. I actually have some local changes that I did not have time to complete (been busy lately).
I'll make sure to add them in the shortest time I can, since they'll provide better code flow.

@omeranha
Copy link
Owner

no problems, you can commit and I can finish if you want, your call
trying to fix a runClient error in my workspace here for now

Wrapped repetitive getEnabledModule() calls into convenience functions.
@omeranha
Copy link
Owner

i'm getting this error for some reason, main is working fine
Module named org.objectweb.asm.tree.analysis was already on the JVMs module path loaded from ... but class-path contains it at location... at [email protected]/cpw.mods.bootstraplauncher.BootstrapLauncher.run(BootstrapLauncher.java:139)

@Achille004
Copy link
Collaborator Author

I've pushed the codeflow improvements (which are complete) and the unfinished change (still compiles and loads fine tho).

@Achille004
Copy link
Collaborator Author

i'm getting this error for some reason, main is working fine Module named org.objectweb.asm.tree.analysis was already on the JVMs module path loaded from ... but class-path contains it at location... at [email protected]/cpw.mods.bootstraplauncher.BootstrapLauncher.run(BootstrapLauncher.java:139)

Do you get this while trying to run the minecraft test? Because if it's the case I don't really know.
I found it easier to test by just creating a neoforge instance with Mekanism (and JEI, Courious and the base stuff) on curseforge and just paste the jar (from jarJar task) into the mods folder, a bit slower but more manageable in my opinion.
(Also doing it like this just loads fine on my client)

@omeranha
Copy link
Owner

omeranha commented Oct 13, 2024

hi, amazing work, everything seems to be working right, now I got some questions
I think we can change the attack unit module hud to also display 0, 50, 100, 150, 200 and 250 instead of 0, 4, 8, 16 and 32, just passing into code to see if it works, or you can do this, just comment here
Thinking about gameplay issues, the items id of mekatana and bow was changed in the pull request, this could leave to potential losses into older world saves to lose the item already made and of course i'm gonna port this changes to 1.20.1 (most playerbase) so I think we need to revert the changes or warn the players somehow

@Achille004
Copy link
Collaborator Author

Achille004 commented Oct 13, 2024

Sorry for the dead silence, my area had a several internet down this weekend.

I think we can change the attack unit module hud to also display 0, 50, 100, 150, 200 and 250 instead of 0, 4, 8, 16 and 32, just passing into code to see if it works, or you can do this, just comment here

Yes I can surely and easily adapt it to a 50*n instead of 2^n. But the real question is, do we actually wanna do that? I used a 2^n scale so that it was as similar as the original Attack Amplification Unit, but with the radial twist.

Thinking about gameplay issues, the items id of mekatana and bow was changed in the pull request, this could leave to potential losses into older world saves to lose the item already made and of course i'm gonna port this changes to 1.20.1 (most playerbase) so I think we need to revert the changes or warn the players somehow

It would be the smartest idea to just revert the changes for 1.20.1, but I'd keep them for 1.21 as it's a major update and I dont actually see people porting between them. Especially because I think that keeping a "naming standard" should be appropriate to line up with Mekanism.

@Achille004
Copy link
Collaborator Author

Achille004 commented Oct 13, 2024

By the way, as I mentioned, I'd say we just keep these changes for the 1.21, and just adapt them to 1.20.1.
Also, I think it could be really useful to mention that I actually have some almost up-to-date 1.20.1 code. I was playing a modpack in this version and this is actually the reason why I even decided to contribute here. So yeah, I'd say we wrap up with 1.21 (since it's the most recent version) and then I'll open a PR to adapt this one to 1.20.1, if that's okay with you.

@omeranha
Copy link
Owner

I think we can change the attack unit module hud to also display 0, 50, 100, 150, 200 and 250 instead of 0, 4, 8, 16 and 32, just passing into code to see if it works, or you can do this, just comment here

Yes I can surely and easily adapt it to a 50*n instead of 2^n. But the real question is, do we actually wanna do that? I used a 2^n scale so that it was as similar as the original Attack Amplification Unit, but with the radial twist.

my only thing about is the damage scale that appears in module tweaker and/in-game damage amplification hud, it's just a detail that some players may not get it instant, what do you think? like its easier to "setup" the damage your want instead of the damage scale

…e multiplied by the index of the current mode.

Created convenience functions to get the base damage and energy usage based on the ItemStack given.
Implemented lombok to increase code readability.
@Achille004 Achille004 mentioned this pull request Oct 19, 2024
@Achille004
Copy link
Collaborator Author

Achille004 commented Oct 19, 2024

I've updated the code as you asked. Now it shows 0, 50, 100, etc... and I've also been able to link it with the base damage!
Now we just need to address the creative mode condition and I'd say it's all set.
Also, as you can see, I've moved the two feature proposed in the TODOs to a proper place.

@Achille004
Copy link
Collaborator Author

I've tried to implement the creative mode condition, but I had no success.
I cleaned up the code and I'd say it's finally ready.

@omeranha
Copy link
Owner

hello, planning to merge this PR tomorrow and update the mod to 1.21.1 finally, do we got anything to do? LGTM

@Achille004
Copy link
Collaborator Author

The code is tested and ready for a merge.
I have added some ideas into #26.

@omeranha omeranha merged commit 23ad687 into omeranha:main Nov 15, 2024
@omeranha
Copy link
Owner

published in curseforge, thanks for your time and help in the project
now the idea is backport it to 1.20.1

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.

2 participants