Conversation
|
Add/Remove Preferences methods are not referencing a specific firearm, so there is no reason for them being on instance. Imagine creating a firearm in Host inventory to modify the preferences of a player... |
IRacle1
left a comment
IRacle1
left a comment
There was a problem hiding this comment.
bro cooked with setting ammo🥹
In general terms, as i see new ammo system first time, i dont liked even idea of it to be such moduled and working on abstractions. but anyway it is😭
Firstly, code is kinda scary for thing that should be elemental. I originally wanted to separate the ammo modules and make this api more low-level, so that people would choose what they need based on the essence of it. But in the long run this approach is probably not a good thing. What you wrote can't logically be shorter(even if you remove the logic behind the changed sign, if removing ammo: first from magazine second from barrels, else barrels -> magazine, it still be very complex)
Second, and main thing, you dont handle PumpActionModule as a another barrel module, rr did you have some reason?
Actually, I have no reason to reject it by architecture reasons
Either way, I'll wait for other devs opinions
| // Magazines that contain most of the ammo and can be reloaded | ||
| List<IPrimaryAmmoContainerModule> primaryContainers = ListPool<IPrimaryAmmoContainerModule>.Pool.Get(Base.Modules.OfType<IPrimaryAmmoContainerModule>()); |
There was a problem hiding this comment.
there is a point of why you using a list of primary ammocontainer? currently weapons have only one
There was a problem hiding this comment.
There is nothing preventing plugins from adding a second one, NW code supports that as far as I know
There was a problem hiding this comment.
Is it real? if it's true then good, but each module has its own id, network code. And main thing the modules list itself is an array😭😭😭
|
It so happens that PumpActionModule can store ammo in TWO FUCKING WAYS because NW are too lazy to give shotguns a separate ammo container. It's a 200-line setter now, should we maybe make some kind of a separate class to deal with this shit? |
well, that's the reason of adding abstractions partially, they create many interfaces for modules, but still we have to go up to the lower level of abstraction to do things😓 |
|
There is still an issue with Ammo setter that it cant set revolver ammo over its max size, but everything else works fine |
|
It's unfortunately impossible to have more ammo in the revolver than the maximum amount its mag can store by default. This is because each space for a bullet is a separate network object with its own feelings (mainly about being loaded or not) |
|
@IRacle1 there is a problem with MaxAmmo |
2 participants
Description
Describe the changes
The current version of Exiled.API.Features.Items.Firearm is not working correctly, this PR is supposed to fix some/all of the problems
Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
No
Types of changes
Submission checklist
Other