[suggestion] expose bullet damage and flight speed multipliers to datapacks/configuration
Colin-J-Wood opened this issue ยท 16 comments
I enjoy playing with this mod, but the bullet speed is simply too low to be very usable in PVP and iron bullets aren't very much of an upgrade for damage. I would like to suggest exposing the base values of the projectile entity's speed to either a configuration file or a datapack.
Note I have previously attempted to make a datapack that extracts its Motion vector and multiplies it. This failed miserably, not sure if it was a maths error, but when the projectile would near the ground it would kill itself before it could even land on an entity, and never actually multiplied its motion.
I understand you have provided "that should be an addon" in the past, but that is simply technically way too difficult for other developers to pull off, as configuration should be the job of the base mod, not an addon. the reason for this is how unreasonable it would be to create mixins just to expose your classes properly to forge's config system.
This could also help boost performance in the case of a pvp server, as bullets could land much faster, resulting in less bullets being mid flight at any given moment, if I was provided the opportunity to multiply the speed in the ways I am asking for.
Bullets are slow because something in the velocity/firing/angle gets messed up when the speed is too high. When I tested higher velocities the firing angle would get messed up and it would "snap" at certain angles, which made it impossible to aim.
That's why the sniper bullets aren't that much faster, and why I don't really want to make it tweakable unless I find a fix.
I don't really have time these days to make a proper config though, but making a bandaid addon that adds a "vanilla" bullet with the damage you want should be easy enough.
I have taken a look at your code and found that you are using single floats when spawning a projectile. this is bad news as Minecraft actually calculates values of angles and velocities as doubles, which you seemingly converted to post calculation, wasting accuracy.
I recommend rewriting the maths you have so that they work with doubles and never cast, the snapping to angle sounds like a math rounding error related to sin/cos
Okay so I have a few devs and we are preparing a fork of this codebase for 1.16.5 to add multiple things:
iron/gold tiers of sniper/shotgun/gatling, and diamond version of pistol, with values that make each a nice investment/upgrade
a configuration file system that changes the values of fire rate, damage multiplier, accuracy, speed on each gun, and damage on each bullet.
backporting fixes of the projectile inheriting the vertical velocity of the player.
fixing the issue with projectiles being unstable past a speed of 4.
what are your thoughts on this? would you accept the PR or do you advise us to publish it as a fork? we understand your mod has the MIT license although it is also copyrighted, we ask on what grounds is the mod copyrighted, in the event you will not publish our pr to curseforge
I'll happily "merge" the fix for unstable projectiles at higher velocities (as in copy the fix and credit whoever managed to do it with huge thanks, unless someone actually PRs it to the 1.18 branch) since it stumped me a lot and I'd love to know what is causing the issue.
As for the other changes, I'm not planning on updating anything on 1.16 and would have probably said no to the other changes anyway (especially the tier changes one since it's against how I designed the mod). Given that, feel free to publish your fork as its own mod. I'd just ask a few things if you do:
- Make it clear it's a fork
- Link to the original mod somewhere on the page (something simple like "This is a fork of 1.16.5 Guns Without Roses[link]")
- Like plaster something on the logo/name so people don't mix it up with the original
- Keep the credit for the textures you'll still use (since they're made by mcvinnyq)
- Don't add me on the project page if it gets on CF/Modrinth
Thanks for asking about it first, and have a nice day all of you on the fork!
The packet handling for motion literally forces a cap at 4, causing the client to desync the location irregardless of speed. what is required to fix this is to create a completely custom packet using mixins. you can check our fork out here when he's done pushing everything that he had to do.
Since I'm fine with the projectile speed I can live with that, just won't get faster snipers. The baseline collision detection for bullets that go through wall (Meet Your Fight's Phantasmal Rifle) is also borked up in a way that I can't fix without overriding the whole default collision behavior on the projectile, so I'll just add it to the list of things that would need a fully custom entity (or clever mixins).
It was also discovered that testing for the entity's kill range with getDeltaMovement().lengthSqrt() for whatever reason actually modifies the resulting value in the base class itself post-calculation, causing the "snapping to angle" behaviour that coincided with the rendering faults. don't use this method. just give the projectile a kill timer, or find some other method to get distance from player instead.
I'm only using this method when testing for the kill velocity (bullets get slowed down in water, this makes them despawn when they are near a stop) and not for any kill range. Bullets also already have a kill timer. Noted though, I'll change how the kill velocity is checked, but I'm curious where exactly does calling that method changes the velocity?
Unless you're saying a vanilla kill range is testing that, in that case I don't know where that would be.
I'm only using this method when testing for the kill velocity (bullets get slowed down in water, this makes them despawn when they are near a stop) and not for any kill range. Bullets also already have a kill timer. Noted though, I'll change how the kill velocity is checked, but I'm curious where exactly does calling that method changes the velocity?
basically, when you getDeltaMovement(), by doing .lengthSqrt(), it applies that mathematics before it returns the result which ends up "restoring" the position client side. this compounds on the floating point result several times, which of course results in a rounding maths error that causes it to deviate instead of being precise with its angle from player. note it's only a client side issue, the hitbox is positioned correctly on the server side (assuming you had a correctly coded class to support above speed 4 that is)
all the measures discussed were implemented by mojang specifically because they could not figure out the mathematics of why things broke past a speed of 4.
you can check the code in our fork in the "network" and "mixin" folders for more information so you can compare them to vanilla code yourself. note we also shifted the mappings to parchment/mojmap.
Okay, so my coder for the project I had on hand, hypherionmc (adriaan), discovered something nasty about vanilla (and consequently forge)'s AbstractProjectileEntity class.
The packet handling for motion literally forces a cap at 4, causing the client to desync the location irregardless of speed. what is required to fix this is to create a completely custom packet using mixins. you can check our fork out here when he's done pushing everything that he had to do.
I know that mixins are scary, but to fix projectile speed, sadly it's the only way. unless you go the route of completely rewriting a projectile entity from the ground up using nothing but the base Entity class.
It was also discovered that testing for the entity's kill range with getDeltaMovement().lengthSqrt() for whatever reason actually modifies the resulting value in the base class itself post-calculation, causing the "snapping to angle" behaviour that coincided with the rendering faults. don't use this method. just give the projectile a kill timer, or find some other method to get distance from player instead.
You can put the "this is a fork" in normal size font, but yeah that's good for me.
We've posted the fork on CurseForge and I would like you to quickly glance over it to make sure we have properly adopted your requests for credit: