From 6a1d2124df6385691963fba916ddbead6f8bfbd2 Mon Sep 17 00:00:00 2001 From: moneyl <8206401+Moneyl@users.noreply.github.com> Date: Thu, 6 Feb 2020 10:13:32 -0500 Subject: [PATCH] =?UTF-8?q?Revert=20unnecessary=20changes=20made=20by=20pr?= =?UTF-8?q?evious=20PR=20to=20SolutionCompon=E2=80=A6=20(#583)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #574 added an out arg to `SolutionComponent.TryRemoveSolution` containing the solution that was removed. I made this change since I thought there was no way to get the removed solution for further use. Didn't notice `SplitSolution` which provides nearly the same behavior. With this PR I want to remove that out arg from `TryRemoveSolution` to keep the SolutionComponent API simple and to avoid having multiple ways of doing things. Existing code uses `SplitSolution`, I just wasn't paying attention. Feel free to deny merging this if I'm over thinking it. I think it'll help keep solution code from becoming a mess. --- .../Components/Chemistry/PourableComponent.cs | 6 ++---- Content.Shared/Chemistry/Solution.cs | 7 +------ .../Components/Chemistry/SolutionComponent.cs | 7 ++----- .../Shared/Chemistry/Solution_Tests.cs | 21 ++++--------------- 4 files changed, 9 insertions(+), 32 deletions(-) diff --git a/Content.Server/GameObjects/Components/Chemistry/PourableComponent.cs b/Content.Server/GameObjects/Components/Chemistry/PourableComponent.cs index f2785ca210..b0f47233b0 100644 --- a/Content.Server/GameObjects/Components/Chemistry/PourableComponent.cs +++ b/Content.Server/GameObjects/Components/Chemistry/PourableComponent.cs @@ -76,11 +76,9 @@ namespace Content.Server.GameObjects.Components.Chemistry _localizationManager.GetString("Container is full")); return false; } - //Remove transfer amount from attacker - if (!attackSolution.TryRemoveSolution(realTransferAmount, out var removedSolution)) - return false; - //Add poured solution to this solution + //Move units from attackSolution to targetSolution + var removedSolution = attackSolution.SplitSolution(realTransferAmount); if (!targetSolution.TryAddSolution(removedSolution)) return false; diff --git a/Content.Shared/Chemistry/Solution.cs b/Content.Shared/Chemistry/Solution.cs index 2255691a1e..bbb9937249 100644 --- a/Content.Shared/Chemistry/Solution.cs +++ b/Content.Shared/Chemistry/Solution.cs @@ -129,11 +129,8 @@ namespace Content.Shared.Chemistry /// Remove the specified quantity from this solution. /// /// The quantity of this solution to remove - /// Out arg. The removed solution. Useful for adding removed solution - /// into other solutions. For example, when pouring from one container to another. - public void RemoveSolution(int quantity, out Solution removedSolution) + public void RemoveSolution(int quantity) { - removedSolution = new Solution(); if(quantity <= 0) return; @@ -141,7 +138,6 @@ namespace Content.Shared.Chemistry if (ratio <= 0) { - removedSolution = this.Clone(); //Todo: Check if clone necessary RemoveAllSolution(); return; } @@ -156,7 +152,6 @@ namespace Content.Shared.Chemistry var newQuantity = (int)Math.Floor(oldQuantity * ratio); _contents[i] = new ReagentQuantity(reagent.ReagentId, newQuantity); - removedSolution.AddReagent(reagent.ReagentId, oldQuantity - newQuantity); } TotalVolume = (int)Math.Floor(TotalVolume * ratio); diff --git a/Content.Shared/GameObjects/Components/Chemistry/SolutionComponent.cs b/Content.Shared/GameObjects/Components/Chemistry/SolutionComponent.cs index 498a070332..d1e3b8fca7 100644 --- a/Content.Shared/GameObjects/Components/Chemistry/SolutionComponent.cs +++ b/Content.Shared/GameObjects/Components/Chemistry/SolutionComponent.cs @@ -127,16 +127,13 @@ namespace Content.Shared.GameObjects.Components.Chemistry /// Attempt to remove the specified quantity from this solution /// /// Quantity of this solution to remove - /// Out arg. The removed solution. Useful for adding removed solution - /// into other solutions. For example, when pouring from one container to another. /// Whether or not the solution was successfully removed - public bool TryRemoveSolution(int quantity, out Solution removedSolution) + public bool TryRemoveSolution(int quantity) { - removedSolution = new Solution(); if (CurrentVolume == 0) return false; - _containedSolution.RemoveSolution(quantity, out removedSolution); + _containedSolution.RemoveSolution(quantity); OnSolutionChanged(); return true; } diff --git a/Content.Tests/Shared/Chemistry/Solution_Tests.cs b/Content.Tests/Shared/Chemistry/Solution_Tests.cs index e2c82b7b4d..cada149224 100644 --- a/Content.Tests/Shared/Chemistry/Solution_Tests.cs +++ b/Content.Tests/Shared/Chemistry/Solution_Tests.cs @@ -141,14 +141,11 @@ namespace Content.Tests.Shared.Chemistry { var solution = new Solution("water", 700); - solution.RemoveSolution(500, out var removedSolution); + solution.RemoveSolution(500); //Check that edited solution is correct Assert.That(solution.GetReagentQuantity("water"), Is.EqualTo(200)); Assert.That(solution.TotalVolume, Is.EqualTo(200)); - //Check that removed solution is correct - Assert.That(removedSolution.GetReagentQuantity("water"), Is.EqualTo(500)); - Assert.That(removedSolution.TotalVolume, Is.EqualTo(500)); } [Test] @@ -156,14 +153,11 @@ namespace Content.Tests.Shared.Chemistry { var solution = new Solution("water", 800); - solution.RemoveSolution(1000, out var removedSolution); + solution.RemoveSolution(1000); //Check that edited solution is correct Assert.That(solution.GetReagentQuantity("water"), Is.EqualTo(0)); Assert.That(solution.TotalVolume, Is.EqualTo(0)); - //Check that removed solution is correct - Assert.That(removedSolution.GetReagentQuantity("water"), Is.EqualTo(800)); - Assert.That(removedSolution.TotalVolume, Is.EqualTo(800)); } [Test] @@ -173,15 +167,11 @@ namespace Content.Tests.Shared.Chemistry solution.AddReagent("water", 1000); solution.AddReagent("fire", 2000); - solution.RemoveSolution(1500, out var removedSolution); + solution.RemoveSolution(1500); Assert.That(solution.GetReagentQuantity("water"), Is.EqualTo(500)); Assert.That(solution.GetReagentQuantity("fire"), Is.EqualTo(1000)); Assert.That(solution.TotalVolume, Is.EqualTo(1500)); - - Assert.That(removedSolution.GetReagentQuantity("water"), Is.EqualTo(500)); - Assert.That(removedSolution.GetReagentQuantity("fire"), Is.EqualTo(1000)); - Assert.That(removedSolution.TotalVolume, Is.EqualTo(1500)); } [Test] @@ -189,13 +179,10 @@ namespace Content.Tests.Shared.Chemistry { var solution = new Solution("water", 800); - solution.RemoveSolution(-200, out var removedSolution); + solution.RemoveSolution(-200); Assert.That(solution.GetReagentQuantity("water"), Is.EqualTo(800)); Assert.That(solution.TotalVolume, Is.EqualTo(800)); - - Assert.That(removedSolution.GetReagentQuantity("water"), Is.EqualTo(0)); - Assert.That(removedSolution.TotalVolume, Is.EqualTo(0)); } [Test]