Refactor/destroy genclosestammo cache class

CombatExtendedRWMod / CombatExtended

Combat Extended mod for RimWorld

128 stars 91 forks source link

Refactor/destroy genclosestammo cache class #1212

Open MadaraUchiha opened 4 years ago

MadaraUchiha commented 4 years ago

Additions

N/A

Changes

Refactors turret reload mechanics

Breaking Changes:

Reloading turrets directly from inventory is not possible anymore. Players need to make the pawn drop from inventory first. This behavior is consistent with vanilla refueling.

References

Todo: Open issue.

Reasoning

Removing the GenClosestAmmo class as suggested by maintainers on #1209.

Alternatives

N/A

Testing

Check tests you have performed:

[ ] Compiles without warnings
[ ] Game runs without errors
[ ] (For compatibility patches) ...with and without patched mod loaded
[ ] Playtested a colony (specify how long)

mitasamodel commented 4 years ago

Hello! Thanks for working on it! I'm trying to test it in my game and the issue seems to be here =( One question. Is it compiled already? The update date for "Assemblies" directory and "Source" directory is not the same. Unfortunately I cannot compile it by myself.

Thanks, Dmitrii

MadaraUchiha commented 4 years ago

@mitasamodel It is not compiled yet. In fact, I am not sure it even compiles! This PR is very much WIP, it's not yet playable.

My other PR #1209 makes an attempt to fix this issue and makes things better but not perfect. You can give that one a try and see if that one works as a crutch until this one is ready :)

mitasamodel commented 4 years ago

@MadaraUchiha yep, I have tried that one (#1209), but the issue was still there. I found a bit other workaround: to be able to reload auto-turrets the pawn should take ammo in the inventory. But as described N7Huntsman, sometimes weird things happen.

Thanks for working on it! :)

MadaraUchiha commented 4 years ago

By the way, @zhrocks11 @N7Huntsman early thoughts on that logger class? I figured such a thing was good to have in general and I didn't find a similar utility.

N7Huntsman commented 4 years ago

@MadaraUchiha C# stuff isn't my wheelhouse, I'm afraid, so I couldn't weigh-in. @NoImageAvailable may have an opinion one way or another.

MadaraUchiha commented 4 years ago

By the way, @zhrocks11 @N7Huntsman early thoughts on that logger class? I figured such a thing was good to have in general and I didn't find a similar utility.

cc @NoImageAvailable

NoImageAvailable commented 4 years ago

The correct C# pattern would be surrounding it with an #if DEBUG but CE also uses it's own set of dev settings in the controller class. That's where in-depth logging parameters live.

MadaraUchiha commented 4 years ago

@NoImageAvailable I had thought of using the compiler preprocessor statements, but that would mean 3 lines for each logging statement, and that's just too sad.

I will look into the in-depth logging setting, thanks!