DolphinWX: Add a filename column #3149

ShadowsFriend · 2015-10-07T09:42:24Z

Adds a filename column to DolphinWX. This was requested in issue https://bugs.dolphin-emu.org/issues/8972.

sepalani · 2015-10-07T11:26:56Z

Please could you provide a screenshot?

Source/Core/DolphinWX/GameListCtrl.cpp

+				return  1 * t;
+			if (iso1->GetFileName() < iso2->GetFileName())
+				return -1 * t;
+			return 0;


JosJuice · 2015-10-07T12:24:54Z

Please write fileName instead of fname throughout the code.
It would be useful if you added file name sorting as a fallback for COLUMN_TITLE sorting, right after the disc number fallback.

ShadowsFriend · 2015-10-07T13:27:50Z

Thanks for the feedback! Here is the requested screenshot:

The other stuff is work in progress now. I've also made a mistake in the resizing part, which I fixed locally. In the process, however, I found that if you hide the column left to size, its contents will be pressed into the size column looking like this (this screenshot being from current master without my patch and the region column unticked):

This probably wasn't found by now because the region column is enabled by default. Because the file column is added to left of size and is hidden by default, this issue would be visible by default with this commit. I suspect autosize of being the culprit. Should I ignore it for now and just work on the file column?

ShadowsFriend · 2015-10-07T14:10:55Z

Just pushed the commit that should implement the requested changes. It also fixes the mistake I made in the resizing part that was mentioned above. It caused the filename column to always be shown even when hidden.
Now there's still that squishing bug from the second screenshot. I found that it doesn't only happen to the size column. It also happens to the filename column when the region column is deactivated.

Source/Core/DolphinWX/GameListCtrl.cpp

+				SetColumnWidth(COLUMN_TITLE, resizable / 3);
+				SetColumnWidth(COLUMN_MAKER, resizable / 3);
+				SetColumnWidth(COLUMN_FILENAME, resizable / 3);
+			}


ShadowsFriend · 2015-10-08T13:52:05Z

In the meantime I found a solution for the bug shown on the second screenshot. Should I add it as a commit to this PR or should I open a different one? Thing is, one line depends on whether this PR will be accepted or not.

JosJuice · 2015-10-08T14:36:39Z

Depends on whether you're comfortable with fixing merge conflicts for either PR when the other is merged, and whether not having the fix will cause problems for this PR. As it is now, it does cause problems. However, I think the File column would fit better visually to the right of Maker, so that all of the long text fields are in one place. In that case, it wouldn't be more of a problem than it was without this PR.

ShadowsFriend · 2015-10-08T15:17:29Z

I've chosen this position only because for me file name and size kind of belong together. But I totally see your point in it being more visually pleasing to right of Maker. I'm going to move it. This hides the issue at least partially. It does, however, reappear when the file column is activated and the Maker column gets autohidden because of the window being too small.

ShadowsFriend · 2015-10-08T15:48:15Z

It's done.

ShadowsFriend · 2015-10-08T16:44:58Z

Just changed strcasecmp to wxStricmp (and therefore included wx/wxcrt.h) to hopefully make the OSX buildbot happy.

Source/Core/DolphinWX/GameListCtrl.cpp

+				    wxFileNameFromPath(iso2->GetFileName()))
+				{
+					return t * wxStricmp(wxFileNameFromPath(iso1->GetFileName()),
+					                     wxFileNameFromPath(iso2->GetFileName()));


ShadowsFriend · 2015-10-11T02:58:03Z

I've decided to add the fix for the squeezing issue as first commit and to adjust the filename column changes to be based upon it.

Source/Core/DolphinWX/GameListCtrl.cpp

 		event.Veto();
+	}
 }


Before the columns of the gamelist were filled with content regardless of their visibility. This led to display bugs when certain columns, for example the region column, were hidden. The first problem was the InsertItemInReportView() function because it refilled all columns with content on every call to update() without checking for their visibility. While this issue would have easily been solved by adding conditionals before each column update, the maker column would have still caused problems for it autohides on resize and those do not call update(). Therefore it was necessary to move the column update logic from InsertItemInReportView() to a new one that allows for seperate modification of an item's columns.

ShadowsFriend · 2015-10-11T03:34:06Z

Thank you for the feedback! The changes have been implemented.

degasus · 2015-12-02T13:51:54Z

LGTM

DolphinWX: Add a filename column

JosJuice reviewed Oct 7, 2015
View reviewed changes

Source/Core/DolphinWX/GameListCtrl.cpp

return 1 * t;

if (iso1->GetFileName() < iso2->GetFileName())

return -1 * t;

return 0;

This comment was marked as off-topic.

Sign in to view

ShadowsFriend force-pushed the fname_column branch from 1eafd45 to 0747868 Compare October 7, 2015 14:03

sepalani reviewed Oct 7, 2015
View reviewed changes

Source/Core/DolphinWX/GameListCtrl.cpp

SetColumnWidth(COLUMN_TITLE, resizable / 3);

SetColumnWidth(COLUMN_MAKER, resizable / 3);

SetColumnWidth(COLUMN_FILENAME, resizable / 3);

}

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

ShadowsFriend force-pushed the fname_column branch from 0747868 to a99e4c5 Compare October 7, 2015 14:33

ShadowsFriend force-pushed the fname_column branch from a99e4c5 to fda48d0 Compare October 8, 2015 15:45

ShadowsFriend force-pushed the fname_column branch 2 times, most recently from 291cc71 to 05311e6 Compare October 8, 2015 16:44

ShadowsFriend force-pushed the fname_column branch 3 times, most recently from 3d750fe to ee2f748 Compare October 11, 2015 02:54

lioncash reviewed Oct 11, 2015
View reviewed changes

Source/Core/DolphinWX/GameListCtrl.cpp

wxFileNameFromPath(iso2->GetFileName()))

{

return t * wxStricmp(wxFileNameFromPath(iso1->GetFileName()),

wxFileNameFromPath(iso2->GetFileName()));

This comment was marked as off-topic.

Sign in to view

lioncash reviewed Oct 11, 2015
View reviewed changes

Source/Core/DolphinWX/GameListCtrl.cpp

event.Veto();

}

}

This comment was marked as off-topic.

Sign in to view

ShadowsFriend added 2 commits October 11, 2015 05:01

DolphinWX: Add a filename column

3fdaf37

ShadowsFriend force-pushed the fname_column branch from ee2f748 to 3fdaf37 Compare October 11, 2015 03:33

ShadowsFriend changed the title ~~Add filename column to DolphinWX~~ DolphinWX: Add a filename column Oct 11, 2015

degasus added a commit that referenced this pull request Dec 2, 2015

Merge pull request #3149 from ShadowsFriend/fname_column

a3433e1

DolphinWX: Add a filename column

degasus merged commit a3433e1 into dolphin-emu:master Dec 2, 2015

ShadowsFriend deleted the fname_column branch December 2, 2015 19:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DolphinWX: Add a filename column #3149

DolphinWX: Add a filename column #3149

ShadowsFriend commented Oct 7, 2015

sepalani commented Oct 7, 2015

This comment was marked as off-topic.

JosJuice commented Oct 7, 2015

ShadowsFriend commented Oct 7, 2015

ShadowsFriend commented Oct 7, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

ShadowsFriend commented Oct 8, 2015

JosJuice commented Oct 8, 2015

ShadowsFriend commented Oct 8, 2015

ShadowsFriend commented Oct 8, 2015

ShadowsFriend commented Oct 8, 2015

This comment was marked as off-topic.

ShadowsFriend commented Oct 11, 2015

This comment was marked as off-topic.

ShadowsFriend commented Oct 11, 2015

degasus commented Dec 2, 2015

DolphinWX: Add a filename column #3149

DolphinWX: Add a filename column #3149

Conversation

ShadowsFriend commented Oct 7, 2015

sepalani commented Oct 7, 2015

This comment was marked as off-topic.

JosJuice commented Oct 7, 2015

ShadowsFriend commented Oct 7, 2015

ShadowsFriend commented Oct 7, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

ShadowsFriend commented Oct 8, 2015

JosJuice commented Oct 8, 2015

ShadowsFriend commented Oct 8, 2015

ShadowsFriend commented Oct 8, 2015

ShadowsFriend commented Oct 8, 2015

This comment was marked as off-topic.

ShadowsFriend commented Oct 11, 2015

This comment was marked as off-topic.

ShadowsFriend commented Oct 11, 2015

degasus commented Dec 2, 2015