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

Added an accuracy field to bows. #4852

Closed
wants to merge 10 commits into from
Closed

Conversation

Mipppy
Copy link

@Mipppy Mipppy commented May 20, 2024

Does what the title says. This adds an extra field onto ranged items, which lets you specify the accuracy of the projectiles shot out. I tested it to oblivion so I don't think it breaks anything.

… not change with inaccuracy. I ended rewriting most of the code I previously wrote, and testing was a nightmare, but I got it done. I learned a lot about the codebase and how to implement other features in the future.
@KlemenDEV
Copy link
Contributor

Hey, just heads up that we are porting to 1.20.6 right now so there may be some delay in merging of PRs

Copy link
Contributor

@KlemenDEV KlemenDEV left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution.

I have left some comments.

As we are currently in the porting phase, expect that you will also need to provide the 1.20.6 code in the future. We may also not be able to provide much mentoring during porting.

I would recommend considering helping with porting right now if you want to do more PRs at this time.

@KlemenDEV KlemenDEV marked this pull request as draft May 21, 2024 19:08
@Mipppy
Copy link
Author

Mipppy commented May 21, 2024

Thank you for being kind to me as I learn how to contribute to open-source projects.
I have adjusted the code based on your comments.
How can I help with porting? Is porting getting new block types, sounds, etc and adding them to a list, or is there a different way of doing things?

@KlemenDEV
Copy link
Contributor

KlemenDEV commented May 21, 2024

Thank you for being kind to me as I learn how to contribute to open-source projects.

No problem, you are more than welcome! ;) Just please in the future don't mark as resolved as this is task of reviewer to mark as resolved if/when they review and consider it done :)

How can I help with porting? Is porting getting new block types, sounds, etc and adding them to a list, or is there a different way of doing things?

Yes, and updating all the code. Check out #4823 and existing pull requests tagged with "[1.20.6]" in title to see how we do this ;)

@KlemenDEV KlemenDEV marked this pull request as ready for review May 21, 2024 20:23
@Mipppy
Copy link
Author

Mipppy commented May 21, 2024

My bad, I thought I was supposed to mark as resolved as I fixed them.

world.addFreshEntity(projectile);
world.playSound(null, entity.getX(), entity.getY(), entity.getZ(), ForgeRegistries.SOUND_EVENTS
.getValue(new ResourceLocation("entity.arrow.shoot")), SoundSource.PLAYERS, 1, 1f / (world.getRandom().nextFloat() * 0.5f + 1));
</#if>

Copy link
Contributor

Choose a reason for hiding this comment

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

This is also unrelated. Applies to both MC versions. Please use https://github.com/MCreator/MCreator/pull/4852/files tab to find such changes.

Please fix all of them, not just the ones I mark

@KlemenDEV KlemenDEV marked this pull request as draft May 22, 2024 07:12
@@ -1052,6 +1052,7 @@ Color.white, new Procedure("condition4"),
item.projectile = new ProjectileEntry(modElement.getWorkspace(),
getRandomDataListEntry(random, ElementUtil.loadArrowProjectiles(modElement.getWorkspace())));
item.shootConstantly = emptyLists;
item.rangedAccuracy = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Test value should differ from default value in UI

@KlemenDEV
Copy link
Contributor

Since we merged another item/projectile related PR, you will need to merge master branch into your branch and fix some of the merge conflicts that happened during this process

@Mipppy
Copy link
Author

Mipppy commented May 23, 2024

Little busy, I'll deal with merge conflicts as soon as I can

@KlemenDEV
Copy link
Contributor

When you have time, no rush :D

Copy link
Contributor

github-actions bot commented Jun 8, 2024

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Do not remove this label manually, it should be removed by the bot when new activity occurs.

@github-actions github-actions bot added the stale Stale issue label Jun 8, 2024
@github-actions github-actions bot closed this Jun 11, 2024
@KlemenDEV
Copy link
Contributor

If you have ever time to continue working on this, feel free to re-open the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Stale issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants