Redo every input configuration dialog

[aldelaro5]

The current layouts of these dialogs are sometime good and a lot of times terrible. The major problem with these is the classes in InputConfigDiag.h, particularly GamepadPage and ControlGroupSizer. The former will create a page with everything in the dialog and put that in a tab, but this is coming from an old layout that got remvoed since then so you get an uneccesary tab. The later however is annoying, it will dynamically generate a sizer that has EVERY control group in it without caring enough about packing them in such a way that the window is presentable and optiomally constructed.

This PR aims to solve this rigidity by allowing InputConfigDialog to be inhereted from the different input dialog and have their specialised constructor that will make the UI. Instead of generating the entire sizer, the only thing generated are the individual input groups and the specialised constructor would decide where to put each groups. This allows many more layout possibilities and flexibility for future modification.

The biggest benefit from this are the hotkey configuration dialog and the wiimote configuration dialog, their window got particularly less wide and overall just more presentable. As for the others however, the chnages are minimal or even absent, but at the very least, if there's need for changes in the future, it would be much easier to do them with this PR.

GamepadPage was dropped in favor of putting the port number in the tittle bar.

Here are the dialogs:

Linux: http://imgur.com/a/A6Fkl
Windows: http://imgur.com/a/sFBjc

For MacOS, @MaJoRoesch said that they looked fine.

I still need to do the extensions dialog (some can benefit from better sizers) which are currently broken rn. I am submitting this as a WIP because it's shaping already to be a huge PR so reviews of what I have done so far woudl be appreciated.

Also because I expect some bikesheding :)

All dialogs are done, this PR being feature complete, it is now ready for full reviews.

---

This change is 

[MayImilae]

Ok, I'll go through each section and give my thoughts.

Hotkeys: YES YES YES YES YES this is what it has needed for SO long!

GC Keyboard: It's a LOT of buttons, but, neatly arranged. I think it can't really get any better, imo.

Emulated Wii Remote: I still don't like how emulated wiimote is designed. That's not your fault, and your arrangement is a lot better, but I think it still feels imposing and difficult. I'd really like to see Buttons, D-Pad, Extensions, rumble, options, and hotkeys in one tab, and tilt, IR, swing, and shake in another tab. Since you get rid of the vestigal tabs for port numbering, it is definitely doable. It would help make it a bit easier to process the motion controls, I think.

Also, opening an extension in macOS does this:

You may want to fix that!

[aldelaro5]

@MaJoRoesch as the PR message says, extensions dialog are broken rn because I still have to do them. The current constructor for them is almsot empty, but it still creates the controlGroupSizer (which I plan to get rid of) so it makes it at 0, 0 from what it seems, aka it's broken until i do them. This is why it's a WIP.

As for the emulated one, I can do tabs if you want.

[mbc07]

Nice work, but wxWidgets on Windows is not happy:

That happens whenever I try to open any Input Config dialog (GC Controller, Wii Remote, Hotkeys, etc)

[aldelaro5]

@mbc07 Can you check now? I think I knew why it did that, if it doesn't I will test on my windows 10 vm.

Btw, this never asserted on linux, great wx....

[mbc07]

Yep, you fixed it, latest version of this PR doesn't trigger any asserts on Windows :)

[MayImilae]

We talked about this on IRC already, but the emu wii remote window is much better! This is a much needed improvement to these UIs, and I look forward to when this is merged!

[mimimi085181]

Increase and decrease IR belong into the hotkey graphics tab, not the wiimote one. IR stands for internal resolution here.

Would it be a good idea to have hotkeys to change the IR pointer region size?

Also, is it intended that the hotkey settings use new and seperate config entries compared to master?

[ghost]

There were some comparable edits made in #4417.

I made a similar comment in that thread but I think some of these windows are really in need of a complete makeover rather than just shuffling the window around.

[aldelaro5]

@PEmu1 that pr modifies ControlGroupSizer and some wiimote group, my pr proposes to completely remove any dynamic generation of UI, except for each group in ControlGroupBoc because it works quite well for what it does. The only limitation is not being able to specify a column count, but I guess with this pr, such addition won't be hard to add as optional parameter to the function.

Like this pr aims to add flexibility for now, make a lot of dialogs better with sucvh flexibility and most importantly, keep the flexibility gain for the future. So even though some edits are indeed the same, I still think there's good benefit for this pr, maybe more improvements can come out of it after.

@mimimi085181 what do you mean separate? they have their own ini name as I saw in the code yes....

As for IR, oops, I though it meant infrared, sorry, I will fix the groups.

[aldelaro5]

@leoetlino I made it much better by having the device and profile selector take more space, but unfortunately, this is the best I could get. The alternative would have been to add spacing.....but it looks a bit weird. I also reduced the size for the keyboard ones too because I could here.

@Helios747 good news, for whatever reasons, doing the above got rid of the spacing so that's nice.

[lioncash]

Review status: 0 of 53 files reviewed at latest revision, 28 unresolved discussions.

---

Source/Core/Core/HotkeyManager.cpp, line 167 at r6 (raw file):

{
  unsigned int group = static_cast<HotkeyManager*>(s_config.GetController(0))->FindGroupByID(id);
  unsigned int groupKey =

group_key

---

Source/Core/Core/HotkeyManager.cpp, line 256 at r6 (raw file):

  {
    std::vector<u32> bitmasks;
    int groupCount = (m_groups_info[group].last - m_groups_info[group].first) + 1;

const int group_count

You can also use this to initialize the vector like so, since the size is known beforehand:

std::vector<u32> bitmasks(group_count);

and then the loop can be

for (size_t key = 0; key < bitmasks.size(); key++)
  bitmasks[key] = static_cast<u32>(1 << key);

---

Source/Core/Core/HotkeyManager.h, line 154 at r6 (raw file):

  HKGP_STATE_MISC,

  NUM_GROUPS,

This should have a name like NUM_HOTKEY_GROUPS or something, to make it more specific, since these constants are in the global scope.

---

Source/Core/Core/HotkeyManager.h, line 178 at r6 (raw file):

  void GetInput(HotkeyStatus* const hk);
  std::string GetName() const override;
  ControlGroup* GetHotkeyGroup(HotkeyGroup group);

All of these functions being added can be const member functions.

---

Source/Core/Core/HotkeyManager.h, line 188 at r6 (raw file):

  std::array<ControlGroup*, NUM_GROUPS> m_hotkey_groups;
  ControlGroup* m_options;
  std::array<HotkeyGroupInfo, NUM_GROUPS> m_groups_info = {

This array can technically be moved to the cpp file. It's not used in a stateful way, since its only used as a lookup table.

either way, this table can be const (it can also be static if it needs to stay in the class definition itself).

---

Source/Core/Core/HW/GCKeyboardEmu.cpp, line 7 at r6 (raw file):

#include "Core/HW/GCKeyboardEmu.h"
#include "Common/Common.h"
#include "InputCommon/KeyboardStatus.h"

You still need to include InputCommon/ControllerEmu.h here.

---

Source/Core/Core/HW/GCPad.h, line 8 at r4 (raw file):

Previously, aldelaro5 (aldelaro5) wrote…

> Ended up needing the header for some methods a bit futher.

You can just include `InputCommon/ControllerEmu.h` instead and move the include to the GCPadEmu include to the cpp file.

---

Source/Core/Core/HW/GCPadEmu.h, line 16 at r6 (raw file):

  MainStick,
  CStick,
  Dpad,

DPad

---

Source/Core/Core/HW/GCPadEmu.h, line 22 at r6 (raw file):

};

class ControlGroup;

outermost forward declarations should be at the top of the file before concrete type definitions.

---

Source/Core/Core/HW/Wiimote.h, line 13 at r6 (raw file):

class InputConfig;
enum class WiimoteGroup;

to properly forward declare these you would do:

namespace WiimoteEmu
{
enum class WiimoteGroup;
enum class NunchukGroup;
enum class ClassicGroup;
enum class GuitarGroup;
enum class DrumsGroup;
}

as the code currently is, it's declaring the existence of enum classes with the same names as the groups in the global namespace, not the ones in WiimoteEmu.

then, the WiimoteEmu include can be moved to the cpp file.

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 29 at r6 (raw file):

namespace WiimoteEmu
{
#pragma pack(push, 1)

This pack should go above the ReportFeatures struct these enums are on top of

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 34 at r6 (raw file):

{
  Buttons,
  Dpad,

DPad

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 36 at r6 (raw file):

  Dpad,
  Shake,
  Ir,

IR

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 59 at r6 (raw file):

  Buttons,
  Triggers,
  Dpad,

DPad

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.cpp, line 138 at r6 (raw file):

}

ControllerEmu::ControlGroup* Classic::GetGroup(WiimoteEmu::ClassicGroup group)

WiimoteEmu:: isn't required here and in the below case statements.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.h, line 10 at r6 (raw file):

#include "Core/HW/WiimoteEmu/WiimoteEmu.h"

enum class ClassicGroup;

This enum declaration needs to be within the WiimoteEmu namespace below to be a valid forward declaration, since that's where the actual enum is defined. This is currently forwarding an enum of the same name in the global scope which doesn't exist.

The include shouldn't be necessary in the header after moving it.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.h, line 23 at r6 (raw file):

  bool IsButtonPressed() const override;

  ControlGroup* GetGroup(WiimoteEmu::ClassicGroup group);

WiimoteEmu:: isn't necessary here, the class itself is already in the WiimoteEmu namespace.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Drums.cpp, line 85 at r6 (raw file):

}

ControllerEmu::ControlGroup* Drums::GetGroup(WiimoteEmu::DrumsGroup group)

WiimoteEmu:: isn't necessary here and in the case statements

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Drums.h, line 10 at r6 (raw file):

#include "Core/HW/WiimoteEmu/WiimoteEmu.h"

enum class DrumsGroup;

This forward declaration needs to be in the WiimoteEmu namespace below. The include shouldn't be necessary after that.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Drums.h, line 23 at r6 (raw file):

  bool IsButtonPressed() const override;

  ControlGroup* GetGroup(WiimoteEmu::DrumsGroup group);

WiimoteEmu:: isn't required here.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Guitar.cpp, line 106 at r6 (raw file):

}

ControllerEmu::ControlGroup* Guitar::GetGroup(WiimoteEmu::GuitarGroup group)

WiimoteEmu:: isn't required here and in the case statements.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Guitar.h, line 10 at r6 (raw file):

#include "Core/HW/WiimoteEmu/WiimoteEmu.h"

enum class GuitarGroup;

This forward declaration needs to be in the WiimoteEmu namespace below. Afterwards the include shouldn't be necessary in the header.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Nunchuk.cpp, line 117 at r6 (raw file):

}

ControllerEmu::ControlGroup* Nunchuk::GetGroup(WiimoteEmu::NunchukGroup group)

WiimoteEmu:: isn't necessary here and in the case statements.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Nunchuk.h, line 10 at r6 (raw file):

#include "Core/HW/WiimoteEmu/WiimoteEmu.h"

enum class NunchukGroup;

This forward declaration needs to be moved into the WiimoteEmu namespace below. This include shouldn't be necessary in the header then.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Turntable.cpp, line 135 at r6 (raw file):

}

ControllerEmu::ControlGroup* Turntable::GetGroup(WiimoteEmu::TurntableGroup group)

WiimoteEmu:: isn't necessary here and in the case statements.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Turntable.h, line 10 at r6 (raw file):

#include "Core/HW/WiimoteEmu/WiimoteEmu.h"

enum class TurntableGroup;

This forward declaration needs to be moved into the WiimoteEmu namespace below. The include shouldn't be necessary in the header then.

---

Source/Core/DolphinWX/Input/InputConfigDiag.cpp, line 69 at r6 (raw file):

  // show config diag, if "none" isn't selected
  if (extension_type == EXT_NUNCHUK)

A switch statement would probably be more appropriate here, since it checks against the same value subsequently.

---

Source/Core/InputCommon/ControllerEmu.h, line 42 at r6 (raw file):

};

enum

This enum should probably be in the emulated Wiimote class, since it's mainly used to index the attachments array.

---

Comments from Reviewable

[aldelaro5]

Review status: 0 of 53 files reviewed at latest revision, 28 unresolved discussions.

---

Source/Core/Core/HotkeyManager.cpp, line 167 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `group_key`

Done.

---

Source/Core/Core/HotkeyManager.cpp, line 256 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> ```cpp > const int group_count > ``` > > You can also use this to initialize the vector like so, since the size is known beforehand: > > ```cpp > std::vector bitmasks(group_count); > ``` > > and then the loop can be > > ```cpp > for (size_t key = 0; key < bitmasks.size(); key++) > bitmasks[key] = static_cast(1 << key); > ```

Done.

---

Source/Core/Core/HotkeyManager.h, line 154 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This should have a name like `NUM_HOTKEY_GROUPS` or something, to make it more specific, since these constants are in the global scope.

Done.

---

Source/Core/Core/HotkeyManager.h, line 178 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> All of these functions being added can be const member functions.

Done.

---

Source/Core/Core/HotkeyManager.h, line 188 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This array can technically be moved to the cpp file. It's not used in a stateful way, since its only used as a lookup table. > > either way, this table can be const (it can also be static if it needs to stay in the class definition itself).

Done.

---

Source/Core/Core/HW/GCKeyboardEmu.cpp, line 7 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> You still need to include `InputCommon/ControllerEmu.h` here.

Done.

---

Source/Core/Core/HW/GCPad.h, line 8 at r4 (raw file):

Previously, lioncash (Mat M.) wrote…

> You can just include `InputCommon/ControllerEmu.h` instead and move the include to the GCPadEmu include to the cpp file.

Done.

---

Source/Core/Core/HW/GCPadEmu.h, line 16 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> DPad

Done.

---

Source/Core/Core/HW/GCPadEmu.h, line 22 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> outermost forward declarations should be at the top of the file before concrete type definitions.

Done.

---

Source/Core/Core/HW/Wiimote.h, line 13 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> to properly forward declare these you would do: > > ```cpp > namespace WiimoteEmu > { > enum class WiimoteGroup; > enum class NunchukGroup; > enum class ClassicGroup; > enum class GuitarGroup; > enum class DrumsGroup; > } > ``` > > as the code currently is, it's declaring the existence of enum classes with the same names as the groups in the global namespace, not the ones in WiimoteEmu. > > then, the WiimoteEmu include can be moved to the cpp file.

Done.

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 29 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This pack should go above the `ReportFeatures` struct these enums are on top of

Done.

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 34 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> DPad

Done.

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 36 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> IR

Done.

---

Source/Core/Core/HW/WiimoteEmu/WiimoteEmu.h, line 59 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `DPad`

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.cpp, line 138 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't required here and in the below case statements.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.h, line 10 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This enum declaration needs to be within the `WiimoteEmu` namespace below to be a valid forward declaration, since that's where the actual enum is defined. This is currently forwarding an enum of the same name in the global scope which doesn't exist. > > The include shouldn't be necessary in the header after moving it. >

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.h, line 23 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't necessary here, the class itself is already in the WiimoteEmu namespace.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Drums.cpp, line 85 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't necessary here and in the case statements

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Drums.h, line 10 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This forward declaration needs to be in the WiimoteEmu namespace below. The include shouldn't be necessary after that.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Drums.h, line 23 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't required here.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Guitar.cpp, line 106 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't required here and in the case statements.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Guitar.h, line 10 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This forward declaration needs to be in the `WiimoteEmu` namespace below. Afterwards the include shouldn't be necessary in the header.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Nunchuk.cpp, line 117 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't necessary here and in the case statements.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Nunchuk.h, line 10 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This forward declaration needs to be moved into the `WiimoteEmu` namespace below. This include shouldn't be necessary in the header then.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Turntable.cpp, line 135 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> `WiimoteEmu::` isn't necessary here and in the case statements.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Turntable.h, line 10 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This forward declaration needs to be moved into the `WiimoteEmu` namespace below. The include shouldn't be necessary in the header then.

Done.

---

Source/Core/DolphinWX/Input/InputConfigDiag.cpp, line 69 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> A switch statement would probably be more appropriate here, since it checks against the same value subsequently.

Done. idk you coudl declare variables in them if you put braces lol.

---

Source/Core/InputCommon/ControllerEmu.h, line 42 at r6 (raw file):

Previously, lioncash (Mat M.) wrote…

> This enum should probably be in the emulated Wiimote class, since it's mainly used to index the attachments array.

Done.

---

Comments from Reviewable

[lioncash]

Review status: 0 of 53 files reviewed at latest revision, 6 unresolved discussions.

---

Source/Core/Core/HotkeyManager.cpp, line 221 at r7 (raw file):

}

static const std::array<HotkeyGroupInfo, NUM_HOTKEY_GROUPS> m_groups_info = {

this shouldn't have an m_ prefix, since it's not a member variable anymore. Also, const translation unit variables are static by default.

---

Source/Core/Core/HW/Wiimote.h, line 21 at r7 (raw file):

enum class TurntableGroup;
}
class PointerWrap;

You'll want to move this forward declaration underneath InputConfig to keep the same-scoped declarations together.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.cpp, line 153 at r7 (raw file):

    return m_right_stick;
  default:
    return nullptr;

Just a thought, but would it not make more sense to also assert(false); before the return, considering it's clearly a logic error if this point is reached? This applies to other similar switch statements as well.

---

Source/Core/DolphinWX/Input/InputConfigDiag.cpp, line 74 at r7 (raw file):

  case WiimoteEmu::EXT_NUNCHUK:
  {
    NunchukInputConfigDialog dlg(this, m_config, "Nunchuk Configuration", m_port_num);

These title strings should be enclosed like _("string here")to allow them to be translated.

---

Source/Core/DolphinWX/Input/InputConfigDiag.h, line 199 at r7 (raw file):

public:
  InputConfigDialog(wxWindow* const parent, InputConfig& config, const wxString& name,
                    const int port_num = 0);

Just thought of this, but there should be a virtual destructor below the constructor, since this is used as a base class.

Just placing:

virtual ~InputConfigDialog() = default;

under it should be fine.

---

Source/Core/InputCommon/ControllerEmu.h, line 207 at r7 (raw file):

  public:
    Buttons(const std::string& _name);
    Buttons(const std::string& _name, const std::string& _ui_name);

I might have missed this in the previous review, but please don't prefix variable names with underscores (even if older code does so).

---

Comments from Reviewable

[aldelaro5]

Review status: 0 of 53 files reviewed at latest revision, 6 unresolved discussions.

---

Source/Core/Core/HotkeyManager.cpp, line 221 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> this shouldn't have an m_ prefix, since it's not a member variable anymore. Also, `const` translation unit variables are `static` by default.

Done.

---

Source/Core/Core/HW/Wiimote.h, line 21 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> You'll want to move this forward declaration underneath InputConfig to keep the same-scoped declarations together.

Done.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.cpp, line 153 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> Just a thought, but would it not make more sense to also `assert(false);` before the return, considering it's clearly a logic error if this point is reached? This applies to other similar switch statements as well.

Done.

---

Source/Core/DolphinWX/Input/InputConfigDiag.cpp, line 74 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> These title strings should be enclosed like `_("string here")`to allow them to be translated.

Done.

---

Source/Core/DolphinWX/Input/InputConfigDiag.h, line 199 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> Just thought of this, but there should be a virtual destructor below the constructor, since this is used as a base class. > > Just placing: > > ```cpp > virtual ~InputConfigDialog() = default; > ``` > > under it should be fine.

Done.

---

Source/Core/InputCommon/ControllerEmu.h, line 207 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> I might have missed this in the previous review, but please don't prefix variable names with underscores (even if older code does so).

Done.

---

Comments from Reviewable

[lioncash]

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.cpp, line 153 at r7 (raw file):

Previously, aldelaro5 (aldelaro5) wrote…

> Done.

You need to include `` to ensure the `assert` macro is always available.

---

Comments from Reviewable

[aldelaro5]

Review status: 0 of 53 files reviewed at latest revision, 1 unresolved discussion.

---

Source/Core/Core/HW/WiimoteEmu/Attachment/Classic.cpp, line 153 at r7 (raw file):

Previously, lioncash (Mat M.) wrote…

> You need to include `` to ensure the `assert` macro is always available.

Done.

---

Comments from Reviewable

[aldelaro5]

@leoetlino Found a way to fix the buttons size while maintining relatively absent or veeerrrry low spacing.

I pushed some proportions changes for the top part of some windows, they now have very little or none spacing on hidpi and it allowed me to do the above.

For the spinner size, it might be possible to fix them, but due to how involved it sound and how imo it seems to stray away a bit too far from my original intent of this pr, I decided that I would fix it in a separate pr, it was also an issue since a long time.

Finally, it was decided to not change the alignement of the threshold spinners.

[MayImilae]

What's the status of this PR? Is it ready to go, or does it need more work?

[aldelaro5]

@MaJoRoesch All issues and tests have been addressed and done, there's not much holding this pr for merge.

I am just waiting on someone to tell if this should or should not be merged.

[lioncash]

Review status: 0 of 53 files reviewed at latest revision, 2 unresolved discussions.

---

Source/Core/DolphinWX/Input/InputConfigDiag.h, line 237 at r8 (raw file):

  void GetProfilePath(std::string& path);
  ControllerEmu* GetController();

This can be a const member function.

---

Source/Core/DolphinWX/Input/InputConfigDiag.h, line 239 at r8 (raw file):

  ControllerEmu* GetController();

  wxComboBox* profile_cbox;

This should also probably be nullptr by default if device_cbox is.

---

Comments from Reviewable

[aldelaro5]

Review status: 0 of 53 files reviewed at latest revision, 2 unresolved discussions.

---

Source/Core/DolphinWX/Input/InputConfigDiag.h, line 237 at r8 (raw file):

Previously, lioncash (Mat M.) wrote…

This can be a const member function.

Done.

---

Source/Core/DolphinWX/Input/InputConfigDiag.h, line 239 at r8 (raw file):

Previously, lioncash (Mat M.) wrote…

This should also probably be nullptr by default if device_cbox is.

Done.

---

Comments from Reviewable

[MayImilae]

The very empty progress report is coming up on us. This looks ready to go, can someone merge it please? @Helios747 ?