Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix some memory leaks from recreating UserControls #1044

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions POEApi.Model/POEModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,22 @@ public List<Character> GetCharacters(string realm)
return GetProperObjectFromTransport<List<Character>>(Transport.GetCharacters(realm));
}

public List<Item> GetInventory(string characterName, bool forceRefresh, string accountName, string realm)
public List<Item> GetInventory(string characterName, int index, bool forceRefresh, string accountName, string realm)
{
try
{
onStashLoaded(POEEventState.BeforeEvent, index, -1);

if (downOnlyMyCharacters && !Settings.Lists["MyCharacters"].Contains(characterName))
return new List<Item>();

Inventory item = GetProperObjectFromTransport<Inventory>(Transport.GetInventory(characterName, forceRefresh, accountName, realm));
Inventory item = null;
using (var stream = Transport.GetInventory(characterName, forceRefresh, accountName, realm))
{
item = GetProperObjectFromTransport<Inventory>(stream);
}

onStashLoaded(POEEventState.AfterEvent, index, -1);

if (item?.Items == null)
return new List<Item>();
Expand Down
2 changes: 1 addition & 1 deletion POEApi.Model/Stash.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ private void refreshCharacterTab(POEModel currentModel, int tabId, string accoun
var charTab = Tabs.First(t => t.i == tabId);

var characterName = charTab.Name;
var characterItems = currentModel.GetInventory(characterName, true, accountName, realm);
var characterItems = currentModel.GetInventory(characterName, tabId, true, accountName, realm);
var characterStashItems = CharacterStashBuilder.GetCharacterStashItems(characterName, characterItems, tabId + 1);

items.AddRange(characterStashItems);
Expand Down
5 changes: 4 additions & 1 deletion Procurement/Controls/Equipped.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,10 @@ void Equipped_Loaded(object sender, RoutedEventArgs e)

private void render()
{
equipped = new EquipedItems(ApplicationState.Model.GetInventory(Character, false, ApplicationState.AccountName, ApplicationState.CurrentRealm).Where(i => i.InventoryId != "MainInventory"));
// TODO: Get the correct tabId to use (instead of -1).
equipped = new EquipedItems(ApplicationState.Model.GetInventory(Character, -1, false,
ApplicationState.AccountName, ApplicationState.CurrentRealm).Where(
i => i.InventoryId != "MainInventory"));
davinci.Children.Clear();
Dictionary<string, Item> itemsAtPosition = equipped.GetItems();

Expand Down
4 changes: 3 additions & 1 deletion Procurement/Controls/Inventory.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,9 @@ void Inventory_Loaded(object sender, RoutedEventArgs e)

private void refresh(string accountName)
{
this.Invent = ApplicationState.Model.GetInventory(Character, false, accountName, ApplicationState.CurrentRealm).Where(i => i.InventoryId == "MainInventory").ToList();
// TODO: Get the correct tabId to use (instead of -1).
this.Invent = ApplicationState.Model.GetInventory(Character, -1, false, accountName,
ApplicationState.CurrentRealm).Where(i => i.InventoryId == "MainInventory").ToList();
inventByLocation = Invent.ToDictionary(item => new Tuple<int, int>(item.X, item.Y));
render();
}
Expand Down
5 changes: 5 additions & 0 deletions Procurement/View/RecipeView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,10 @@ public RecipeView()
{
get { return this.ViewContent; }
}

public void RefreshRecipes()
{
(this.DataContext as RecipeResultViewModel).RefreshRecipes();
}
}
}
8 changes: 7 additions & 1 deletion Procurement/View/RefreshView.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

namespace Procurement.View
{
public partial class RefreshView : UserControl
public partial class RefreshView : UserControl, IView
{
private StatusController statusController;

Expand All @@ -19,6 +19,11 @@ public RefreshView()
InitializeComponent();
}

public new Grid Content
{
get { return this.ViewContent; }
}

public void RefreshAllTabs()
{
RefreshTabs(true);
Expand Down Expand Up @@ -67,6 +72,7 @@ public void RefreshTabs(bool refreshAllTabs)
ScreenController.Instance.ReloadStash();
ScreenController.Instance.RefreshRecipeScreen();
ScreenController.Instance.UpdateTrading();
statusController.Clear();
}
});
}
Expand Down
11 changes: 10 additions & 1 deletion Procurement/ViewModel/LoginWindowViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,19 @@ private IEnumerable<Item> LoadCharacterInventoryItems(Character character, bool
if (!offline)
_statusController.DisplayMessage((string.Format("Loading {0}'s inventory...", character.Name)));

// TODO: It looks like the character's fake stash tab does not exist at this point. Confirm, and determine
// the best course of action at this point.
var tab = ApplicationState.Stash[character.League].Tabs.FirstOrDefault(
t => t.Name == character.Name && t.IsFakeTab);
int tabId = 0;
if (tab != null)
tabId = tab.i;

List<Item> inventory;
try
{
inventory = ApplicationState.Model.GetInventory(character.Name, false, ApplicationState.AccountName, ApplicationState.CurrentRealm);
inventory = ApplicationState.Model.GetInventory(character.Name, tabId, false, ApplicationState.AccountName,
ApplicationState.CurrentRealm);
success = true;
}
catch (WebException)
Expand Down
7 changes: 6 additions & 1 deletion Procurement/ViewModel/RecipeResultViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public Item SelectedItem
public RecipeResultViewModel()
{
manager = new RecipeManager();
ApplicationState.LeagueChanged += new System.ComponentModel.PropertyChangedEventHandler(ApplicationState_LeagueChanged);
ApplicationState.LeagueChanged += ApplicationState_LeagueChanged;
updateResults();
}

Expand Down Expand Up @@ -72,6 +72,11 @@ private void updateResults()
ItemFilterUpdater.UpdateLootFilters();
}

public void RefreshRecipes()
{
updateResults();
}

void ApplicationState_LeagueChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
{
updateResults();
Expand Down
23 changes: 11 additions & 12 deletions Procurement/ViewModel/ScreenController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public bool ButtonsVisible
private const string INVENTORY_VIEW = "Inventory";
private const string SETTINGS_VIEW = "Settings";
private const string ABOUT_VIEW = "About";
private const string REFRESH_VIEW = "Refresh";

public static ScreenController Instance = null;
private UserControl _selectedView;
Expand Down Expand Up @@ -92,23 +93,22 @@ private void initScreens()
screens.Add(SETTINGS_VIEW, new SettingsView());
screens.Add(RECIPE_VIEW, null);
screens.Add(ABOUT_VIEW, new AboutView());
screens.Add(REFRESH_VIEW, new RefreshView());
}));
}

public void InvalidateRecipeScreen()
{
screens[RECIPE_VIEW] = null;
}

public void RefreshRecipeScreen()
{
Application.Current.Dispatcher.BeginInvoke(DispatcherPriority.Normal,
new Action(() =>
{
// TODO: Cause the RecipeResultsViewModel in the RecipeView to refresh its recipes, instead of
// recreating the RecipeView object. This could perhaps be done by triggering an event, or
// reaching into the view/viewmodel and calling it directly (but that's probably very bad form).
screens[RECIPE_VIEW] = new RecipeView();
// TODO: See if we can find a more elegant way to refresh receipes, instead of calling a method on
// the view. Perhaps something with events?
RecipeView view = screens[RECIPE_VIEW] as RecipeView;
if (view == null)
screens[RECIPE_VIEW] = new RecipeView();
else
view.RefreshRecipes();
}));
}

Expand Down Expand Up @@ -151,14 +151,14 @@ public void LoadView(IView view)
public void LoadRefreshView()
{
ButtonsVisible = false;
SelectedView = new RefreshView();
SelectedView = screens[REFRESH_VIEW] as RefreshView;
(SelectedView as RefreshView).RefreshAllTabs();
}

public void LoadRefreshViewUsed()
{
ButtonsVisible = false;
SelectedView = new RefreshView();
SelectedView = screens[REFRESH_VIEW] as RefreshView;
(SelectedView as RefreshView).RefreshUsedTabs();
}

Expand All @@ -167,7 +167,6 @@ public void ReloadStash()
Application.Current.Dispatcher.BeginInvoke(DispatcherPriority.Normal,
new Action(() =>
{
screens[STASH_VIEW] = new StashView();
SelectedView = screens[STASH_VIEW] as UserControl;
ButtonsVisible = true;
}));
Expand Down
10 changes: 10 additions & 0 deletions Procurement/ViewModel/StatusController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@ public void NotOK()
CheckAccessAndInvoke(() => displayResult("ER", Brushes.Red));
}

public void Clear()
{
CheckAccessAndInvoke(() => ClearInternal());
}

private void ClearInternal()
{
(statusBox.Document.Blocks.LastBlock as Paragraph).Inlines.Clear();
}

public void DisplayMessage(string message)
{
CheckAccessAndInvoke((Action)delegate()
Expand Down
27 changes: 22 additions & 5 deletions Procurement/ViewModel/TabViewModel/StashViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,11 @@ public SortedDictionary<string, int> GemDistribution
public ICommand RefreshCommand => new RelayCommand(x =>
{
ScreenController.Instance.LoadRefreshView();
ScreenController.Instance.InvalidateRecipeScreen();
});

public ICommand RefreshUsedCommand => new RelayCommand(x =>
{
ScreenController.Instance.LoadRefreshViewUsed();
ScreenController.Instance.InvalidateRecipeScreen();
});

public static DateTime LastAutomaticRefresh { get; protected set; }
Expand All @@ -155,7 +153,6 @@ public void OnClientLogFileChanged(object sender, ClientLogFileEventArgs e)
if (ScreenController.Instance.ButtonsVisible)
{
ScreenController.Instance.LoadRefreshViewUsed();
ScreenController.Instance.InvalidateRecipeScreen();
}
}));
}
Expand All @@ -182,6 +179,8 @@ public StashViewModel(StashView stashView)
else
configuredOrbType = (OrbType)Enum.Parse(typeof(OrbType), currencyDistributionMetric);

ApplicationState.Model.StashLoading += ApplicationState_StashLoading;

if (Settings.UserSettings.Keys.Contains(_enableTabRefreshOnLocationChangedConfigName))
{
var enabled = false;
Expand All @@ -194,6 +193,23 @@ public StashViewModel(StashView stashView)
}
}

private void ApplicationState_StashLoading(POEModel sender, POEApi.Model.Events.StashLoadedEventArgs e)
{
if (e.State != POEApi.Model.Events.POEEventState.AfterEvent)
return;

foreach (var tabAndContent in tabsAndContent)
{
if (tabAndContent.Index == e.StashID && tabAndContent.Stash is AbstractStashTabControl)
{
// This tab has been refreshed. We mark it as not ready, so it is refreshed the next time it is
// selected.
(tabAndContent.Stash as AbstractStashTabControl).Ready = false;
break;
}
}
}

private void getAvailableItems()
{
try
Expand Down Expand Up @@ -236,7 +252,6 @@ void tabControl_SelectionChanged(object sender, SelectionChangedEventArgs e)
var item = stashView.tabControl.SelectedItem as TabItem;
selectedTab = item;
Image i = item.Header as Image;
CroppedBitmap bm = (CroppedBitmap)i.Source;
Tab tab = (Tab)i.Tag;
item.Header = StashHelper.GenerateTabImage(tab, true);
}
Expand All @@ -250,6 +265,8 @@ private void unselectPreviousTab(TabItem selectedTab)

void ApplicationState_LeagueChanged(object sender, PropertyChangedEventArgs e)
{
tabsAndContent.Clear();

getAvailableItems();
stashView.tabControl.SelectionChanged -= new SelectionChangedEventHandler(tabControl_SelectionChanged);
stashView.tabControl.Items.Clear();
Expand Down Expand Up @@ -323,9 +340,9 @@ private void closeAndSelect(ContextMenu menu, MenuItem menuItem)

void stashView_Loaded(object sender, RoutedEventArgs e)
{
var stash = ApplicationState.Stash[ApplicationState.CurrentLeague];
for (var i = 1; i <= ApplicationState.Stash[ApplicationState.CurrentLeague].NumberOfTabs; i++)
{
var stash = ApplicationState.Stash[ApplicationState.CurrentLeague];
var currentTab = stash.Tabs[i - 1];

var item = new TabItem
Expand Down
6 changes: 3 additions & 3 deletions Tests/POEApi.Model.Tests/PoeModelTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void GetInventoryTest()
{
_mockTransport.Setup(m => m.GetInventory("", false, "", Realm.PC)).Returns(stream);

var inventory = _model.GetInventory("", false, "", Realm.PC);
var inventory = _model.GetInventory("", -1, false, "", Realm.PC);

Assert.IsNotNull(inventory);
}
Expand Down Expand Up @@ -273,7 +273,7 @@ public void GetPantheonSoulInventoryTest()
using (var stream = GenerateStreamFromString(fakeInventoryInfo))
{
_mockTransport.Setup(m => m.GetInventory(string.Empty, false, string.Empty, Realm.PC)).Returns(stream);
var inventory = _model.GetInventory(string.Empty, false, string.Empty, Realm.PC);
var inventory = _model.GetInventory(string.Empty, -1, false, string.Empty, Realm.PC);

inventory.Should().NotBeNull();
inventory.Should().HaveCount(3);
Expand Down Expand Up @@ -435,7 +435,7 @@ public void GetInventoryWithQuestItemsTest()
{
_mockTransport.Setup(m => m.GetInventory("", false, "", Realm.PC)).Returns(stream);

var inventory = _model.GetInventory("", false, "", Realm.PC);
var inventory = _model.GetInventory("", -1, false, "", Realm.PC);

Assert.IsNotNull(inventory);

Expand Down