Unknown025 / Flans-Mod-Plus

Revision of Flan's Mod Plus for 1.7.10
Other
25 stars 19 forks source link

Small reload cancel improvements #197

Closed pixelrider2000 closed 6 months ago

pixelrider2000 commented 6 months ago

Changes:

GoldSloth commented 6 months ago

Code looks good, however during testing I found an bug. Pretty easy to reproduce, just get a gun and hold right click, and it continues to reload over and over without ammo showing up.

Also, if you spam 'R' ammo will appear in the inventory without any reloading animation taking place. https://streamable.com/fg52u4

GoldSloth commented 6 months ago

https://streamable.com/aqpids

Sorry, another one. Right clicking after reloading cancels reload animation here too.

Ignore this I was using an out of date version.

pixelrider2000 commented 6 months ago

So the holding right click thing was caused by the QueuedReload ticking one tick more than the actual reload time, so you could initiate another reload in that one extra tick and therefore cancel the previous.

About the other 'bug'. I don't think it is a bug. When a reload is finished, you get the ammo that was previously in the gun back. In creative mode you can therefore spam reload and keep getting ammo since no ammo is consumed when you reload. It is only just very noticeable on FULLY LOADED shotguns, because their reload time is (pretty much?) 0. So you spam 'R' and keep getting ammo.

Maybe there is an argument to be made that you shouldn't get the old magazine back in creative mode, but idk...

GoldSloth commented 6 months ago

For the second one the bug is more that the reload animation doesn't play I think

pixelrider2000 commented 6 months ago

I believe that's also not new. IIrc reload time is 0 when no shells are inserted. There being no animation would make sense in that case.

pixelrider2000 commented 6 months ago

float reloadTime = singlesReload ? (type.getReloadTime(gunStack) / maxAmmo) * reloadCount : type.getReloadTime(gunStack);

Yeah. When reloadCount is 0 (shotgun is already full), reloadTime is also 0.

GoldSloth commented 6 months ago

Huh, interesting. The main reason you'd do that is to swap out shells I guess? Perhaps that's something we can add... Even just for a full reload when it's full already and ignore the vase of partial reload

pixelrider2000 commented 6 months ago

Yeah I don't think getReloadTime works properly since it doesn't take partial shot gun reloads into account. There is no need to call this method anyway since reloadTime is already known, so I just replaced it.

This is why you had to wait the full reload before animations would play client-side

GoldSloth commented 6 months ago

Yeah that makes sense. Do you mind making another test build for this?

pixelrider2000 commented 6 months ago

Until now, I didn't even now what forceReload was, so I didn't know it was important :)