Skip to content
This repository has been archived by the owner on Apr 14, 2020. It is now read-only.

(bugfix) Fix bug where turrets cannot be loaded #1209

Draft
wants to merge 53 commits into
base: Development
Choose a base branch
from

Conversation

MadaraUchiha
Copy link
Contributor

Additions

N/A

Changes

Adjustments to other features made in this merge, e.g.

  • Increase turret ammo search radius from 40 tiles to 100 tiles.
  • Fix bug where GenClosestAmmo cache hit would cause the TTL to never be updated.

References

Reasoning

  • Turrets are generally found on the edge of your base, far away from your stockpiles. Hence, the increase in search radius. It may be prudent to add a reason to why a turret can't be reloaded next time (something along the lines of "No compatible ammo in range")

Alternatives

Remove the GenClosestAmmo class, as suggested by @NoImageAvailable.

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 (1 hour, though not very extensively)

Robin Chang and others added 30 commits March 27, 2020 22:37
Old value was too powerful compared with a Lion bite (!)
Increased the sidearm money to ensure more frequent sidearms.
Boosted AUGS' rate of fire to match descriptions, and to distinguish it some more. Slightly boosted range.
Increased the range of the AUG based on effective range of 500m
Adjusted the sight effectiveness for some of the older weapons to make them consistent with new weapons sight effectiveness.
Technically this should never show up in 1.1, but for some reason people seem to be running the latest CE dev bracnh in RW 1.0...
N7Huntsman and others added 17 commits April 5, 2020 16:15
…tches

Vanilla Expanded Weapons Compat Patch Updates
This reverts commit 4022ec3.
Remove from O21 Races branch.
….1Patch

Update to (Rim) Contractor's Arsenal
…Patch1.1

Patch update for Dragonian Race 1.1
…-Races

Patch additional O21 forgotten realms Races
Fix wrong comment
- Also increase ammo search range from 40 tiles to 100 tiles.
- Fix bug where lastSurrdoundingAmmoCheck would not be updated if
GenClosestAmmo cache hit.
- Fixes CombatExtendedRWMod#1188
@MadaraUchiha MadaraUchiha requested review from a team as code owners April 9, 2020 18:13
@NoImageAvailable
Copy link
Collaborator

What's the reasoning behind the try-catch block?

@N7Huntsman
Copy link
Contributor

N7Huntsman commented Apr 9, 2020

We certainly appreciate the help, though unfortunately the issue persists. In testing, I'm seeing some truly bizarre behavior, but I'm hopeful you'll find the details helpful. I've included video of my testing, and a trace for the reload job error I encountered.
Mod list: Harmony (from Steam), Core, Royalty, Combat Extended from your 'reload fix' branch.
Video: https://www.youtube.com/watch?v=TzJw2h3qvGU&feature=youtu.be
Trace from video: https://pastebin.com/Q7AUjSvS
Another, slightly different trace (not in the video, seems to be from a different part of the same job?): https://pastebin.com/9E0bQhcE
Observations:

  • The reloading hauling job seems to get "attached" to a particular stack of ammo for a turret, and won't reload if that particular stack isn't available, even if other stacks of the same type are. Even if the ammo type is changed, they will reload from the "attached" stack, and the turrets ammo type will be set to the type of the "attached" stack.
  • From other (unrecorded) testing, when manning the turret, colonists it will continue to reload from the "attached" stack so long as the stack has enough ammo in it to fill the turrets magazine. Once it is too low to refill the magazine, it will no longer reload from it, and will not switch to another stack.
  • If a colonist has ammo used by the turret in their inventory, if they are selected and the turret is right-clicked, they will instantly drop the ammo from their inventory, even if assigned to it by a loadout.

@NoImageAvailable NoImageAvailable changed the base branch from master to Development April 9, 2020 21:13
@NoImageAvailable
Copy link
Collaborator

All those issues sound like issues with the caching system. Unless there is some 1.1 specific reason it has to be there it should be rolled back.

@MadaraUchiha
Copy link
Contributor Author

What's the reasoning behind the try-catch block?

The variable tracking the last check should be updated regardless of whether the cache lookup bails early. I could have added the last check update before every return, but figured this was easier and clearer.

I do agree that if there's no real good reason for this cache class, it should be removed.

@NoImageAvailable
Copy link
Collaborator

So here's the case for reverting the cache class
Con:

  • It saves some performance on colonies with lots of turrets
  • Simply reverting it will wipe out Mechanoid reloading along with it

Pro:

  • It's extremely buggy and will likely take extended effort to make work
  • Turret reloading isn't a particularly huge performance drain to begin with
  • It introduces lag in reloads even if it works

@MadaraUchiha can you ditch the caching in favor of 1.0 style reloading while keeping mech functionality intact (potentially by reimplementing it in a more reasonable format such as a ThinkNode)?

@MadaraUchiha
Copy link
Contributor Author

MadaraUchiha commented Apr 10, 2020

My RimWorld specific C# experience is very much lacking, but I'll give it my best shot. If you have any resources that you think can be useful (i.e. save me on reading decompiled RimWorld code) please let me know :).

@NoImageAvailable NoImageAvailable marked this pull request as draft April 10, 2020 07:43
@N7Huntsman
Copy link
Contributor

N7Huntsman commented Apr 10, 2020

I'll ping @zhrocks11 on this for the sake of bringing him into the loop so he's aware that @MadaraUchiha will be taking over the turret issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Turrets cannot be reloaded // Turrets reloaded with wrong ammo type
6 participants