-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
Exclusive SuperWeapon Sidebar #1384
base: develop
Are you sure you want to change the base?
Exclusive SuperWeapon Sidebar #1384
Conversation
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
looks like nobody interested for this, so it won't change again until somebody notice it. |
Looks nice but due to the large list of SW that can stack in that side of the screen can be added a button for show/hide this SuperWeapon Sidebar ? |
That's not really true, we're just yet to give feedback, and the situation with you and @CrimRecya working on the same feature is a bit... odd
|
As useful as this may seem to some I still think both this and #1379 are infinitely less desirable than an additional sidebar tab for superweapons. I know there are some potential challenges in trying to implement that but maybe there's some sort of shortcut to 'fake it' rather than trying to plug it into the existing tab system e.g a button that changes the tab display to something that looks like it but isn't and only display's SW's. If I had more time and energy I would help you guys in the research but alas no can do. |
I think that a separate sidebar can also be an alternative option, doesn't have to choose either or. |
of course |
There's no special reason, it's just because I don't know it well enough |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, I am happy with this implementation. Looking at the new button, it looks well implemented to me. Kudos.
It's a shame we have to reimplement all this Ares crap, but eh, not much that can be done.
|
||
if ((int)flags & (int)GadgetFlag::LeftRelease && this->IsPressed) | ||
{ | ||
this->IsPressed = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC when you handle an input, you should exlude it before passing it on.
So flags &= ~GadgetFlag::LeftRelease
or smth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you wanna make it even better, maybe take out the static stuff outside of TacticalButtonClass, and into some new "SWSidebarClass". It's a bit weird to have it in the button itself, even if this is the only purpose of the button.
Just don't forget to resolve conflics and update docs.
Actually, I don't know how to handle conflicts outside of the browser |
You need to rebase your branch onto the latest develop. Then you'll get conflicts inside the files, and you can resolve them in VS/VS Code/other diff program. |
idk how to do it :( |
e4c0b68
to
54ec5e9
Compare
…om/NetsuNegi/Phobos into origin/branch/exclusive-sw-sidebar
TODO: |
It's too hard for me to deal with S/L... |
I don't think you ever need to save/load anything in the sidebar. The game just recreates the sidebar on each load, so you could do the same. |
I will do that in next 2 days. |
But I still need to save/load the superIndex per buttons I think. |
I don't think you need to. Just treat loading as you will treat starting a new game. I haven't looked in depth at how you implemented it, but what I would do is add the superweapon to the SuperWeapon Sidebar when the same superweapon is added to the main sidebar, so you will automatically keep them in sync. Keep in mind that I didn't check in depth how it works, so this is only in theory. |
It seems that the game did not re add cameos after reading the save file. |
It should be finished now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not full review, just randomly stumbled upon something to change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took a brief look and haven't test yet.
我简单看了下,还没测试。
|
||
for (int idx = 1; idx <= 10; idx++) | ||
{ | ||
sprintf_s(buffer, "SW Sidebar Shortcuts Num %02d", idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you forgot to change this to the shortcut key name you defined.
我想你应该是忘记把这里改成你所定义的快捷键名称了。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% on what's up here, please verify if the string you make is what you intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% on what's up here, please verify if the string you make is what you intended.
This should be the Name
of the command class.
This is when loading/saving shortcut keys. Finding the newly added SW shortcut keys by comparing the names, extract their shortcut key codes and translate to text through that huge map. Finally display them conveniently on the screen for players.
However, the game originally had a function to display shortcut key text in the settings interface, but I couldn't find it, so this became a form of separate recording and self translation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% on what's up here, please verify if the string you make is what you intended.
This should be the
Name
of the command class. This is when loading/saving shortcut keys. Finding the newly added SW shortcut keys by comparing the names, extract their shortcut key codes and translate to text through that huge map. Finally display them conveniently on the screen for players. However, the game originally had a function to display shortcut key text in the settings interface, but I couldn't find it, so this became a form of separate recording and self translation.
61EF70
is the function.
void WWUI::Get_Keyboard_Key_String(int key, wchar_t *buffer)
|
||
for (int idx = 1; idx <= 10; idx++) | ||
{ | ||
sprintf_s(buffer, "SW Sidebar Shortcuts Num %02d", idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same.
同上。
if (!pSWExt->SW_ShowCameo) | ||
return true; | ||
|
||
if (!Phobos::UI::ExclusiveSWSidebar || Unsorted::ArmageddonMode) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is ArmageddonMode and how is it relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x6A630C
bool SWColumnClass::Clicked(DWORD* pKey, GadgetFlag flags, int x, int y, KeyModifier modifier) | ||
{ | ||
for (const auto button : this->Buttons) | ||
{ | ||
if (button->Clicked(pKey, flags, x, y, modifier)) | ||
return true; | ||
} | ||
|
||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why a column is a ControlClass, and manually fires the Clicked function for buttons?
Why not just have the buttons fire it themselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because I want it to block mouse action.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And buttons click has been blocked by Column's, so I manually check it.
void SWSidebarClass::RecordHotkey(int buttonIndex, int key) | ||
{ | ||
const int index = buttonIndex - 1; | ||
|
||
if (this->KeyCodeData[index] == key) | ||
return; | ||
|
||
this->KeyCodeData[index] = key; | ||
std::wostringstream oss; | ||
|
||
if (key & static_cast<int>(WWKey::Shift)) | ||
oss << KeyboardCodeTextMap[static_cast<int>(WWKey::Shift)] << L"+"; | ||
|
||
if (key & static_cast<int>(WWKey::Ctrl)) | ||
oss << KeyboardCodeTextMap[static_cast<int>(WWKey::Ctrl)] << L"+"; | ||
|
||
if (key & static_cast<int>(WWKey::Alt)) | ||
oss << KeyboardCodeTextMap[static_cast<int>(WWKey::Alt)] << L"+"; | ||
|
||
const int pureKey = key & 0xFF; | ||
|
||
if (KeyboardCodeTextMap.contains(pureKey)) | ||
oss << KeyboardCodeTextMap[pureKey]; | ||
else | ||
oss << L"Unknown"; | ||
|
||
this->KeyCodeText[index] = oss.str(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of this, as well as the huge map above? Why did vanilla settings not suit you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just copy from PR#1379.
// I cannot add it into YRppp :( | ||
// It always failed, help me | ||
static void __fastcall HouseClass_UpdateSuperWeaponsUnavailable(HouseClass* pHouse) | ||
{ | ||
JMP_STD(0x50B1D0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ask in the chat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added HouseClass::AISupers() for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please utilize HouseClass::AISuper()
DEFINE_HOOK(0x533E69, UnknownClass_sub_533D20_LoadKeyboardCodeFromINI, 0x6) | ||
{ | ||
GET(CommandClass*, pCommand, ESI); | ||
GET(int, key, EDI); | ||
|
||
const char* name = pCommand->GetName(); | ||
char buffer[29]; | ||
|
||
for (int idx = 1; idx <= 10; idx++) | ||
{ | ||
sprintf_s(buffer, "SW Sidebar Shortcuts Num %02d", idx); | ||
|
||
if (!_strcmpi(name, buffer)) | ||
SWSidebarClass::Global()->RecordHotkey(idx, key); | ||
} | ||
|
||
return 0; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What why? The game will load keyboard commands by itself as long as you add them to the Commands vector.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How can I get the hotkey of the command?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to get the hotkey of the command, the command is self-sufficientr (just does everything in ::Execute)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to get the hotkey of the command, the command is self-sufficientr (just does everything in ::Execute)
This is not for us to execute, this is for players to see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not for us to execute, this is for players to see.
Alright, we see now.
I have not looked in depth at the algorithm you do, but what I know is that in the hotkey configuration window the game already writes the key combination like Alt+A
for example. What I think we should do: when writing the label at the cameo of superweapon, see the order of it in the sidebar, look up the corresponding hotkey from the global array of registered hotkeys (by type) and transform the key combination that is used for that hotkey into a string, then print it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not for us to execute, this is for players to see.
Alright, we see now.
I have not looked in depth at the algorithm you do, but what I know is that in the hotkey configuration window the game already writes the key combination like
Alt+A
for example. What I think we should do: when writing the label at the cameo of superweapon, see the order of it in the sidebar, look up the corresponding hotkey from the global array of registered hotkeys (by type) and transform the key combination that is used for that hotkey into a string, then print it.
So how can I do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So how can I do that?
@ZivDero is currently in process of adding stuff to YRpp so you can accomplish that, please wait a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So how can I do that?
@ZivDero is currently in process of adding stuff to YRpp so you can accomplish that, please wait a bit.
Alright
{ | ||
if (SWSidebarClass::IsEnabled()) | ||
{ | ||
if (const auto button = SWSidebarClass::Global()->CurrentButton) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Questionable syntax, avoid assignments inside if blocks (aside from dynamic_cast, perhaps)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a shorthand for a null check and is actually quite convenient. There is no risk here because you assign a variable and not compare. We do that often in Phobos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer not doing this since it's not transformative (a dynamic_cast would at least give you something else, in this case you can just repeat this thing 3 times inside which is fine IMO, or declare a variable inside the block. Unjustified use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well either way my point is that it's not necessarily bad. This is a personal preferences territory in this case IMO.
src/Commands/ToggleSWSidebar.cpp
Outdated
const wchar_t* ToggleSWSidebar::GetUIName() const | ||
{ | ||
return GeneralUtils::LoadStringUnlessMissing("TXT_TOGGLE_SW_SIDEBAR", L"Toggle SuperWeapon Sidebar"); | ||
} | ||
|
||
const wchar_t* ToggleSWSidebar::GetUICategory() const | ||
{ | ||
return CATEGORY_INTERFACE; | ||
} | ||
|
||
const wchar_t* ToggleSWSidebar::GetUIDescription() const | ||
{ | ||
return GeneralUtils::LoadStringUnlessMissing("TXT_TOGGLE_SW_SIDEBAR_DESC", L"Toggle SuperWeapon Sidebar."); | ||
} | ||
|
||
void ToggleSWSidebar::Execute(WWKey eInput) const | ||
{ | ||
ToggleSWButtonClass::SwitchSidebar(); | ||
|
||
if (SidebarExt::Global()->SWSidebar_Enable) | ||
MessageListClass::Instance->PrintMessage(GeneralUtils::LoadStringUnlessMissing("TXT_EX_SW_BAR_VISIBLE", L"Set exclusive SW sidebar visible."), RulesClass::Instance->MessageDelay, HouseClass::CurrentPlayer->ColorSchemeIndex, true); | ||
else | ||
MessageListClass::Instance->PrintMessage(GeneralUtils::LoadStringUnlessMissing("TXT_EX_SW_BAR_INVISIBLE", L"Set exclusive SW sidebar invisible."), RulesClass::Instance->MessageDelay, HouseClass::CurrentPlayer->ColorSchemeIndex, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please have someone proofread your default strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's kinda what we are for :P
MakeCommand<ToggleSWSidebar>(); | ||
|
||
MakeCommand<FireTacticalSWCommandClass<0>>(); | ||
MakeCommand<FireTacticalSWCommandClass<1>>(); | ||
MakeCommand<FireTacticalSWCommandClass<2>>(); | ||
MakeCommand<FireTacticalSWCommandClass<3>>(); | ||
MakeCommand<FireTacticalSWCommandClass<4>>(); | ||
MakeCommand<FireTacticalSWCommandClass<5>>(); | ||
MakeCommand<FireTacticalSWCommandClass<6>>(); | ||
MakeCommand<FireTacticalSWCommandClass<7>>(); | ||
MakeCommand<FireTacticalSWCommandClass<8>>(); | ||
MakeCommand<FireTacticalSWCommandClass<9>>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be enough for commands to appear in the settings and save/load in/from keyboard.ini. Why the other fluff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there
const auto it = std::find_if(buttons.begin(), buttons.end(), [superIdx](TacticalButtonClass* const button) { return button->SuperIndex == superIdx; }); | ||
|
||
if (it == buttons.end()) | ||
return false; | ||
|
||
AnnounceInvalidPointer(SWSidebarClass::Global()->CurrentButton, *it); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it
is a weird choice. Maybe button
?
Code2Text[0x28] = L"Down"; | ||
Code2Text[0x29] = L"Select"; | ||
Code2Text[0x2A] = L"Print"; | ||
Code2Text[0x2B] = L"Execute"; | ||
Code2Text[0x2C] = L"PrintScreen"; | ||
Code2Text[0x2D] = L"Insert"; | ||
Code2Text[0x2E] = L"Delete"; | ||
Code2Text[0x2F] = L"Help"; | ||
Code2Text[0x30] = L"0"; | ||
Code2Text[0x31] = L"1"; | ||
Code2Text[0x32] = L"2"; | ||
Code2Text[0x33] = L"3"; | ||
Code2Text[0x34] = L"4"; | ||
Code2Text[0x35] = L"5"; | ||
Code2Text[0x36] = L"6"; | ||
Code2Text[0x37] = L"7"; | ||
Code2Text[0x38] = L"8"; | ||
Code2Text[0x39] = L"9"; | ||
|
||
Code2Text[0x41] = L"A"; | ||
Code2Text[0x42] = L"B"; | ||
Code2Text[0x43] = L"C"; | ||
Code2Text[0x44] = L"D"; | ||
Code2Text[0x45] = L"E"; | ||
Code2Text[0x46] = L"F"; | ||
Code2Text[0x47] = L"G"; | ||
Code2Text[0x48] = L"H"; | ||
Code2Text[0x49] = L"I"; | ||
Code2Text[0x4A] = L"J"; | ||
Code2Text[0x4B] = L"K"; | ||
Code2Text[0x4C] = L"L"; | ||
Code2Text[0x4D] = L"M"; | ||
Code2Text[0x4E] = L"N"; | ||
Code2Text[0x4F] = L"O"; | ||
Code2Text[0x50] = L"P"; | ||
Code2Text[0x51] = L"Q"; | ||
Code2Text[0x52] = L"R"; | ||
Code2Text[0x53] = L"S"; | ||
Code2Text[0x54] = L"T"; | ||
Code2Text[0x55] = L"U"; | ||
Code2Text[0x56] = L"V"; | ||
Code2Text[0x57] = L"W"; | ||
Code2Text[0x58] = L"X"; | ||
Code2Text[0x59] = L"Y"; | ||
Code2Text[0x5A] = L"Z"; | ||
Code2Text[0x5B] = L"LWin"; | ||
Code2Text[0x5C] = L"RWin"; | ||
Code2Text[0x5D] = L"Menu"; | ||
|
||
Code2Text[0x60] = L"Num0"; | ||
Code2Text[0x61] = L"Num1"; | ||
Code2Text[0x62] = L"Num2"; | ||
Code2Text[0x63] = L"Num3"; | ||
Code2Text[0x64] = L"Num4"; | ||
Code2Text[0x65] = L"Num5"; | ||
Code2Text[0x66] = L"Num6"; | ||
Code2Text[0x67] = L"Num7"; | ||
Code2Text[0x68] = L"Num8"; | ||
Code2Text[0x69] = L"Num9"; | ||
Code2Text[0x6A] = L"Num*"; | ||
Code2Text[0x6B] = L"Num+"; | ||
Code2Text[0x6C] = L"Separator"; | ||
Code2Text[0x6D] = L"Num-"; | ||
Code2Text[0x6E] = L"Num."; | ||
Code2Text[0x6F] = L"Num/"; | ||
Code2Text[0x70] = L"F1"; | ||
Code2Text[0x71] = L"F2"; | ||
Code2Text[0x72] = L"F3"; | ||
Code2Text[0x73] = L"F4"; | ||
Code2Text[0x74] = L"F5"; | ||
Code2Text[0x75] = L"F6"; | ||
Code2Text[0x76] = L"F7"; | ||
Code2Text[0x77] = L"F8"; | ||
Code2Text[0x78] = L"F9"; | ||
Code2Text[0x79] = L"F10"; | ||
Code2Text[0x7A] = L"F11"; | ||
Code2Text[0x7B] = L"F12"; | ||
|
||
Code2Text[0x90] = L"NumLock"; | ||
Code2Text[0x91] = L"ScrollLock"; | ||
|
||
Code2Text[0xBA] = L";"; | ||
Code2Text[0xBB] = L"="; | ||
Code2Text[0xBC] = L","; | ||
Code2Text[0xBD] = L"-"; | ||
Code2Text[0xBE] = L"."; | ||
Code2Text[0xBF] = L"/"; | ||
Code2Text[0xC0] = L"`"; | ||
|
||
Code2Text[0xDB] = L"["; | ||
Code2Text[0xDC] = L"\\"; | ||
Code2Text[0xDD] = L"]"; | ||
Code2Text[0xDE] = L"'"; | ||
|
||
Code2Text[static_cast<int>(WWKey::Shift)] = L"Shift"; | ||
Code2Text[static_cast<int>(WWKey::Ctrl)] = L"Ctrl"; | ||
Code2Text[static_cast<int>(WWKey::Alt)] = L"Alt"; | ||
|
||
return Code2Text; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could use the vanilla functions that provide the name of the key, but one would need to add it it YRpp first. For now I guess it's not a big deal, although perhaps maybe it could be taken out of here and into some place as a util?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but one would need to add it it YRpp first
@ZivDero didn't you already do exactly that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but one would need to add it it YRpp first
@ZivDero didn't you already do exactly that?
I encountered some issues the details of which I don't recall and I don't think it ever got to completion, no.
// I cannot add it into YRppp :( | ||
// It always failed, help me | ||
static void __fastcall HouseClass_UpdateSuperWeaponsUnavailable(HouseClass* pHouse) | ||
{ | ||
JMP_STD(0x50B1D0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please utilize HouseClass::AISuper()
return false; | ||
} | ||
|
||
void SWSidebarClass::Init_Clear() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void SWSidebarClass::Init_Clear() | |
void SWSidebarClass::InitClear() |
|
||
for (int idx = 1; idx <= 10; idx++) | ||
{ | ||
sprintf_s(buffer, "SW Sidebar Shortcuts Num %02d", idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% on what's up here, please verify if the string you make is what you intended.
ExclusiveSuperWeaponSidebar
is true.ExclusiveSWSidebar.Interval
specific how many leptons between two columns.ExclusiveSuperWeaponSidebar.Max
controls the maximum number of icons on the leftmost side, which also depends on the current game resolution.ExclusiveSWSidebar.MaxColumn
controls that maximum count of columns.In
uimd.ini
:In
rulesmd.ini