Implement in game command list by xGuTeK · Pull Request #222 · ko4life-net/ko

xGuTeK · 2024-05-17T14:22:44Z

Screenshots

Thanks to @twostars and @stevewgr for asm code :)

twostars · 2024-05-19T02:38:49Z

    D3DCOLOR            FontColor() { return m_crFont; }
    BOOL                FontIsBold() { return m_bFontBold; }
    BOOL                FontIsItalic() { return m_bFontItalic; }
+    CN3UIString *       GetChildStrFromList(std::string str);


This should ideally be a const reference, since we have no reason to copy or change the string, i.e.:

const std::string & str

twostars · 2024-05-19T02:40:18Z

    bool CommandToggleWalkRun();
    bool CommandToggleUISkillTree();
+    bool CommandToggleCmdList();
+    bool OpenCmdEdit(std::string msg);


This should also ideally be a const reference, i.e.:

const std::string & msg

twostars · 2024-05-19T02:42:43Z

+
+//////////////////////////////////////////////////////////////////////
+
+class CUICmdEditDlg : public CN3UIBase {


Officially the class name is simply named CUICmdEdit, I assume based on the leftover comments in the source file, you originally created it with the 'Dlg' suffix and then renamed it without renaming the class itself.

I don't know if this project intends to keep official class names for things like this, but if so, this should be renamed CUICmdEdit.

Ideally we keep the same official names. 100% with you @twostars.

twostars · 2024-05-19T02:44:16Z

+
+//////////////////////////////////////////////////////////////////////
+
+class CUICmdListDlg : public CN3UIBase {


As with the other UI class, officially the class name is simply named CUICmdList.

I don't know if this project intends to keep official class names for things like this, but if so, this should be renamed CUICmdList.

twostars · 2024-05-19T02:45:48Z

+
+class CUICmdListDlg : public CN3UIBase {
+    bool               m_bDestoyDlgAlive;
+    class CUICmdEdit * m_pUICmdEdit;


In its current state this forward declaration is wrong. You named it CUICmdEditDlg (which is unofficial), so if that's the name you're sticking with, it should be forward declared as such as well.

If that gets changed though, of course it's fine to leave.

twostars · 2024-05-19T02:48:42Z

+    std::string command;
+    m_pList_Cmds->GetString(cmdSel, command);
+
+    if (command == "PM") {


There's a couple of issues with doing this like this.
For one, you have the command names defined in the TBL, but then you're explicitly checking (and case-sensitive at that) it against "PM" here.

For another, it's not the only command which officially shows this prompt.

It would be saner to translate this back to either the command type from the enum, or alternatively the resource ID. Officially they do the former.

twostars · 2024-05-19T02:51:51Z

 }

+CN3UIString * CN3UIList::GetChildStrFromList(std::string str) {
+    for (std::list<CN3UIString *>::iterator it = m_ListString.begin(); it != m_ListString.end(); ++it) {


This code is fine, and this is just a nitpick really, but this is a perfect use of auto, to not have to deal with the tedious templated type names, e.g.:

for (auto it = m_ListString.begin(); it != m_ListString.end(); ++it) {

I should also note that the preferable modern way to handle a loop like this is with their new for each loop syntax, e.g.:

for (CN3UIString* pString : m_ListString) {

Although in this particular case I don't know what this project is intending to support, since this is a newer language feature.

Agree with @twostars here. The existing instances ideally we won't refactor in the same PR, just to make the PR with isolated changes, but when adding stuff, let's try and stick to modern C++.

twostars · 2024-05-19T02:53:47Z

+    for (int i = 0; i < 8; i++) {
+        ::_LoadStringFromResource(i + 7800, szCategory); //load command categories
+
+        if (CGameBase::s_pPlayer->m_InfoBase.iAuthority == 0 && szCategory == "GM") { // if not gm hide this category


This is another case of relying on the content of the TBLs. Ideally it should be using an enum or some other language-agnostic indicator, that isn't explicitly relying on the contents of the TBL.

twostars · 2024-05-19T02:55:27Z

+    std::map<uint16_t, std::string> m_mapCmds;
+
+  public:
+    void SetVisible(bool bVisible);


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

void SetVisible(bool bVisible) override;

twostars · 2024-05-19T02:55:46Z

+  public:
+    void SetVisible(bool bVisible);
+
+    bool         OnKeyPress(int iKey);


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

bool OnKeyPress(int iKey) override;

twostars · 2024-05-19T02:56:19Z

+    void SetVisible(bool bVisible);
+
+    bool         OnKeyPress(int iKey);
+    virtual void Release();


Ideally this should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

void Release() override;

twostars · 2024-05-19T02:56:29Z

+
+    bool         OnKeyPress(int iKey);
+    virtual void Release();
+    virtual void Tick();


Ideally this should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

void Tick() override;

twostars · 2024-05-19T02:56:46Z

+    bool         OnKeyPress(int iKey);
+    virtual void Release();
+    virtual void Tick();
+    virtual bool ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg);


Ideally this should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

bool ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg) override;

twostars · 2024-05-19T02:56:57Z

+    virtual void Release();
+    virtual void Tick();
+    virtual bool ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg);
+    void         Render();


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

void Render() override;

twostars · 2024-05-19T02:57:13Z

+    void         Render();
+    void         Open();
+    void         Close(bool bByKey = false);
+    bool         Load(HANDLE hFile);


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

bool Load(HANDLE hFile) override;

twostars · 2024-05-19T02:58:15Z

+    bool         ExecuteCommand(uint8_t cmdSel);
+
+    CUICmdListDlg();
+    virtual ~CUICmdListDlg();


This doesn't really need to redefine itself as virtual; to insist that it is overridden virtually, you'd use override instead, like:

~CUICmdListDlg() override;

But this is more of a preference thing in this case and I don't really know where @stevewgr stands on this.

Agree with you @twostars, with the caveat mentioned in this comment: #222 (comment)

twostars · 2024-05-19T02:58:41Z

+    std::string   m_szArg1;
+
+  public:
+    void SetVisible(bool bVisible);


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

void SetVisible(bool bVisible) override;

twostars · 2024-05-19T02:59:28Z

+
+  public:
+    void SetVisible(bool bVisible);
+    void Open(std::string msg);


Since there's no need to copy or modify, this should be passed by const reference, i.e.:

void Open(const std::string & msg);

twostars · 2024-05-19T02:59:45Z

+    void SetVisible(bool bVisible);
+    void Open(std::string msg);
+
+    bool Load(HANDLE hFile);


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

bool Load(HANDLE hFile) override;

twostars · 2024-05-19T03:00:00Z

+    void Open(std::string msg);
+
+    bool Load(HANDLE hFile);
+    bool ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg);


This method is virtual but doesn't indicate it as such. Ideally it should be using override (rather than redefining it as virtual to enforce the overridden virtual method (that way it can error if it's defined incorrectly).
i.e.

bool ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg) override;

twostars · 2024-05-19T03:00:31Z

+    bool ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg);
+
+    CUICmdEditDlg();
+    virtual ~CUICmdEditDlg();


This doesn't really need to redefine itself as virtual; to insist that it is overridden virtually, you'd use override instead, like:

~CUICmdEditrDlg() override;

But this is more of a preference thing in this case and I don't really know where @stevewgr stands on this.

#222 (comment)

twostars · 2024-05-19T03:02:33Z

+
+bool CUICmdEditDlg::ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg) {
+    if (dwMsg == UIMSG_BUTTON_CLICK) {
+        if (pSender->m_szID == "btn_ok") {


We already have the pointer to this button to check against; can just check it directly instead, i.e.:

if (pSender == m_pBtn_Ok)

twostars · 2024-05-19T03:02:54Z

+            return true;
+        }
+
+        if (pSender->m_szID == "btn_cancel") {


We already have the pointer to this button to check against; can just check it directly instead, i.e.:

if (pSender == m_pBtn_Cancel)

twostars · 2024-05-19T03:05:15Z

+
+    return true;
+}
+bool CUICmdListDlg::UpdateCommandList(uint8_t cmdCat) {


There should probably be a newline before this line, separating the 2 methods.

Ah! That's weird that the code formatter didn't fix that. I'll keep that in mind to check for later.

twostars · 2024-05-19T03:05:20Z

+
+    return true;
+}
+bool CUICmdListDlg::ReceiveMessage(CN3UIBase * pSender, DWORD dwMsg) {


There should probably be a newline before this line, separating the 2 methods.

twostars · 2024-05-19T03:06:41Z

+    return true;
+}
+bool CUICmdListDlg::UpdateCommandList(uint8_t cmdCat) {
+    if (m_pList_Cmds == NULL) {


Again, I don't know what we're working with, but I assume we should be moving away from the old NULL definitions and explicitly using C++11's nullptr instead. But @stevewgr can confirm.

Definitely: #222 (comment)

twostars · 2024-05-19T03:10:14Z

+}
+
+void CUICmdListDlg::Render() {
+    if (!m_bVisible) {


This method doesn't really do anything in your implementation.
Officially, it's responsible for rendering the (yellow) selection border of the appropriate list.

If we're keeping it to official behaviour, it should handle this -- but if not, this implementation is pointless and can go.

twostars · 2024-05-19T03:11:52Z

+
+//this_ui_add_start
+bool CUICmdListDlg::OnKeyPress(int iKey) {
+    return CN3UIBase::OnKeyPress(iKey);


This implementation is also pointless in your implementation.
Officially, it's responsible for handling DIK_ESCAPE to close the UI, the various navigation key input (DIK_TAB, DIK_UP, DIK_DOWN) and also for handling selection behaviour (DIK_RETURN, DIK_NUMPADENTER).

twostars · 2024-05-19T03:13:08Z

+}
+
+void CUICmdListDlg::SetVisible(bool bVisible) {
+    CN3UIBase::SetVisible(bVisible);


Officially this also resets the selection when opening (while not already visible).
As an aside, it also syncs up the button in CUICmd.

twostars · 2024-05-19T03:15:06Z

+class CUICmdListDlg : public CN3UIBase {
+    bool               m_bDestoyDlgAlive;
+    class CUICmdEdit * m_pUICmdEdit;
+    CN3UIButton *      m_pBtn_cancel;


Based on the naming scheme of the others, this should probably be m_pBtn_Cancel

twostars · 2024-05-19T03:17:01Z

+    int   m_iRBtnDownOffs;
+    bool  m_bRBtnProcessing;
+
+    enum iCmd {


I feel like the naming of this enum leaves something to be desired, and will more than likely unintentionally clash with variable names used within this class implementation. Probably something like CmdListCategory or something would avoid any chance of a clash (I don't really know the intended naming scheme for enums here though -- personally I'd match them to the client's e_<name> scheme).

twostars · 2024-05-19T03:44:50Z

Great majority of my comments were rather nitpicky, and largely dependent on the intentions of the codebase, so take from that what you will.

I don't know if it's at all useful, but I will explain how this behaviour is handled officially, for reference.

Officially, on CGameProcMain::Init(), it calls what I name CUICmdList::InitCommands(). This has similar behaviour to the behaviour you inlined (maybe it got inlined in 1.264? I don't know if you were referencing this or not).

The thing with this, is it loads commands into an array for each and every category (not a singular array like it used to):

void __stdcall CUICmdList::InitCommands()
{
  // [COLLAPSED LOCAL DECLARATIONS. PRESS KEYPAD CTRL-"+" TO EXPAND]

  v0 = IDS_CMD_WHISPER;
  v1 = CUICmdList::szCommandsPrivate;           // Private (12)
  do
    CGameBase::GetText(v0++, v1++);
  while ( (int)v1 < (int)CUICmdList::ParseChattingCommand::szCmds );
  v2 = IDS_CMD_TRADE;
  v3 = CUICmdList::szCommandsTrade;             // Trade (4)
  do
    CGameBase::GetText(v2++, v3++);
  while ( (int)v3 < (int)CUICmdList::szCommandsKing );
  v4 = IDS_CMD_PARTY;
  v5 = CUICmdList::szCommandsParty;             // Party (5)
  do
    CGameBase::GetText(v4++, v5++);
  while ( (int)v5 < (int)CUICmdList::szCommandsClan );
  v6 = IDS_CMD_JOINCLAN;
  v7 = CUICmdList::szCommandsClan;              // Clan (9)
  do
    CGameBase::GetText(v6++, v7++);
  while ( (int)v7 < (int)&stru_815C10 );
  v8 = IDS_CMD_CONFEDERACY;
  v9 = CUICmdList::szCommandsKnights;           // Knights (3)
  do
    CGameBase::GetText(v8++, v9++);
  while ( (int)v9 < (int)CUICmdList::szCommandsParty );
  v10 = IDS_CMD_VISIBLE;
  v11 = CUICmdList::szCommandsGM;               // GM (21)
  do
    CGameBase::GetText(v10++, v11++);
  while ( (int)v11 < (int)&byte_815A48 );
  v12 = IDS_CMD_GUARD_HIDE;
  v13 = CUICmdList::szCommandsGuardianMonster;  // Guardian Monster (7)
  do
    CGameBase::GetText(v12++, v13++);
  while ( (int)v13 < (int)CUICmdList::szCommandsPrivate );
  v14 = IDS_CMD_KING_ROYALORDER;
  v15 = CUICmdList::szCommandsKing;             // King (7)
  do
    CGameBase::GetText(v14++, v15++);
  while ( (int)v15 < (int)CUICmdList::szCommandsKnights );
}

With these loaded, in what I call CUICmdList::ParseChattingCommand() (called by the original CGameProcMain implementation), it fetches the command parts as normal, then checks for any manually internally defined commands (e.g. /goto, /rental, etc.).

If none of those match, it then scans the various arrays in a particular order, with each category handled by its own method, which I've loosely named like CUICmdList::ProcessCommand_Private(), CUICmdList::ProcessCommand_Trade(), etc.

This then scans if they match, and based on the matching index in the array (which they'd have an enum coupled to), they handle the command appropriately, for example:

  switch ( eCmd )
  {
    case CMD_PRIVATE_WHISPER:
      szID._allocator._byte = v30;
      std::string::_Tidy(&szID, 0);
      std::string::assign(&szID, arg2, strlen(arg2));
      LOBYTE(v38) = 1;
      CGameProcMain::MsgSend_ChatSelectTarget(*(CGameProcMain **)v31, &szID, CHAT_TARGET_SELECT_PRIVATE);
      LOBYTE(v38) = 0;
      std::string::_Tidy(&szID, 1);
      goto LABEL_74;
    case CMD_PRIVATE_TOWN:
      if ( CGameBase::s_pPlayer->_.m_bStun )
        goto LABEL_16;
      if ( 2 * CGameBase::s_pPlayer->_.m_InfoBase.iHP >= CGameBase::s_pPlayer->_.m_InfoBase.iHPMax
        || CGameBase::s_pPlayer->_.m_InfoExt.iZoneCur == 55 )
      {
        strcpy((char *)&data, "H");
        CAPISocket::Send(CGameProcedure::s_pSocket, (BYTE *)&data._allocator, 2);
      }
      else
      {
        data._allocator._byte = v30;
        std::string::_Tidy(&data, 0);
        LOBYTE(v38) = 2;
        CGameBase::GetText(IDS_ERR_GOTO_TOWN_OUT_OF_HP, &data);
        CGameProcMain::MsgOutput(*(CGameProcMain **)v31, &data, 0xFFFF00FF);
        LOBYTE(v38) = 0;
        std::string::_Tidy(&data, 1);
      }
      goto LABEL_74;
    case CMD_PRIVATE_EXIT:
      CGameProcMain::RequestExit(*(CGameProcMain **)v31);
      goto LABEL_74;

Getting back to the list itself, when it's opened it'll reset the selected list to the category list, and toggle the "option" button on CUICmd appropriately.
When it sets the category, it naturally resets the content of the command list, and then depending on the selected (or default) category, it'll go through and fetch appropriately. For example:

        case CMD_GROUP_PRIVATE:
          v3 = 0;
          p_data_long = &CUICmdList::szCommandsPrivate[0]._data_long;
          do
          {
            CGameBase::GetText((e_TextResourceID)(v3 + 8100), &szMsg);
            v5 = *p_data_long;
            if ( !*p_data_long )
              v5 = NewFilename;
            data_long = szMsg._data_long;
            if ( !szMsg._data_long )
              data_long = NewFilename;
            sprintf(Buffer, data_long, v5);
            v7 = *p_data_long;
            if ( !*p_data_long )
              v7 = NewFilename;
            a2._allocator._byte = v56;
            std::string::_Tidy(&a2, 0);
            std::string::assign(&a2, v7, strlen(v7));
            v54 = 0xFF80FF80;
            v57 = (char **)&v53;
            LOBYTE(v75) = 1;
            sub_4A64F0(&v53, Buffer, &v56);
            v8 = v61->_.m_pList_Content;
            LOBYTE(v75) = 1;
            CN3UIList::AddString(v8, &a2, 0xFFC6C6FB, v53, v54);
            LOBYTE(v75) = 0;
            std::string::_Tidy(&a2, 1);
            p_data_long += 4;
            ++v3;
          }
          while ( (int)p_data_long < (int)&CUICmdList::ParseChattingCommand::szCmds[0][4] );
          break;

What they're (predominantly) doing here is they're just using the various command group enums (e.g. private's containing /PM, /TOWN, etc) and looping until they reach the end of this enum. For each of the categories.

Selecting a command predominantly just passes it through, but based on the selected category ID (and then consequently the appropriately selected command, by index in the array -- which is mapped to the previously mentioned enum), it'll determine whether or not it needs to show CUICmdEdit or not.

If it doesn't, it just passes it through to CUICmdList::ParseChattingCommand() to be handled.

CUICmdEdit's behaviour officially utilises CallBackProc to pass the notifications. On enter (specifically just DIK_RETURN, anyway), it'll forward the "1" event to its parent (CUICmdList) for handling.

CUICmdList then handles this in its CUICmdList::CallBackProc() implementation to fetch the command name and args, pass it to CUICmdList::ParseChattingCommand() and finally empties out the contents of CUICmdEdit's edit control.

I think that's about the general gist of how it works.

twostars · 2024-05-19T03:57:42Z

+            pBPC->AnimationAdd(ANI_GREETING2, true); // ????
        } else if (11 == iState) {
-            pBPC->AnimationAdd(ANI_WAR_CRY1, true); // ����
+            pBPC->AnimationAdd(ANI_WAR_CRY0, true); // ????


I realise it's not logical, but officially 11 = ANI_WAR_CRY1, 12 = ANI_WAR_CRY0 and 13 = ANI_WAR_CRY2. I made the exact same mistake.

case STATE_CHANGE_ACTION: switch ( iState ) { case 1u: CPlayerBase::AnimationAdd((CPlayerBase *)pBPC, ANI_GREETING0, 1); break; case 2u: CPlayerBase::AnimationAdd((CPlayerBase *)pBPC, ANI_GREETING1, 1); break; case 3u: CPlayerBase::AnimationAdd((CPlayerBase *)pBPC, ANI_GREETING2, 1); break; case 11u: CPlayerBase::AnimationAdd((CPlayerBase *)pBPC, ANI_WAR_CRY1, 1); break; case 12u: CPlayerBase::AnimationAdd((CPlayerBase *)pBPC, ANI_WAR_CRY0, 1); break; case 13u: CPlayerBase::AnimationAdd((CPlayerBase *)pBPC, ANI_WAR_CRY2, 1); break; } return;

xGuTeK · 2024-05-23T18:26:49Z

Great majority of my comments were rather nitpicky, and largely dependent on the intentions of the codebase, so take from that what you will.

I don't know if it's at all useful, but I will explain how this behaviour is handled officially, for reference.

Officially, on CGameProcMain::Init(), it calls what I name CUICmdList::InitCommands(). This has similar behaviour to the behaviour you inlined (maybe it got inlined in 1.264? I don't know if you were referencing this or not).

The thing with this, is it loads commands into an array for each and every category (not a singular array like it used to):
void __stdcall CUICmdList::InitCommands()
{
  // [COLLAPSED LOCAL DECLARATIONS. PRESS KEYPAD CTRL-"+" TO EXPAND]

  v0 = IDS_CMD_WHISPER;
  v1 = CUICmdList::szCommandsPrivate;           // Private (12)
  do
    CGameBase::GetText(v0++, v1++);
  while ( (int)v1 < (int)CUICmdList::ParseChattingCommand::szCmds );
  v2 = IDS_CMD_TRADE;
  v3 = CUICmdList::szCommandsTrade;             // Trade (4)
  do
    CGameBase::GetText(v2++, v3++);
  while ( (int)v3 < (int)CUICmdList::szCommandsKing );
  v4 = IDS_CMD_PARTY;
  v5 = CUICmdList::szCommandsParty;             // Party (5)
  do
    CGameBase::GetText(v4++, v5++);
  while ( (int)v5 < (int)CUICmdList::szCommandsClan );
  v6 = IDS_CMD_JOINCLAN;
  v7 = CUICmdList::szCommandsClan;              // Clan (9)
  do
    CGameBase::GetText(v6++, v7++);
  while ( (int)v7 < (int)&stru_815C10 );
  v8 = IDS_CMD_CONFEDERACY;
  v9 = CUICmdList::szCommandsKnights;           // Knights (3)
  do
    CGameBase::GetText(v8++, v9++);
  while ( (int)v9 < (int)CUICmdList::szCommandsParty );
  v10 = IDS_CMD_VISIBLE;
  v11 = CUICmdList::szCommandsGM;               // GM (21)
  do
    CGameBase::GetText(v10++, v11++);
  while ( (int)v11 < (int)&byte_815A48 );
  v12 = IDS_CMD_GUARD_HIDE;
  v13 = CUICmdList::szCommandsGuardianMonster;  // Guardian Monster (7)
  do
    CGameBase::GetText(v12++, v13++);
  while ( (int)v13 < (int)CUICmdList::szCommandsPrivate );
  v14 = IDS_CMD_KING_ROYALORDER;
  v15 = CUICmdList::szCommandsKing;             // King (7)
  do
    CGameBase::GetText(v14++, v15++);
  while ( (int)v15 < (int)CUICmdList::szCommandsKnights );
}
With these loaded, in what I call CUICmdList::ParseChattingCommand() (called by the original CGameProcMain implementation), it fetches the command parts as normal, then checks for any manually internally defined commands (e.g. /goto, /rental, etc.).

If none of those match, it then scans the various arrays in a particular order, with each category handled by its own method, which I've loosely named like CUICmdList::ProcessCommand_Private(), CUICmdList::ProcessCommand_Trade(), etc.

This then scans if they match, and based on the matching index in the array (which they'd have an enum coupled to), they handle the command appropriately, for example:
  switch ( eCmd )
  {
    case CMD_PRIVATE_WHISPER:
      szID._allocator._byte = v30;
      std::string::_Tidy(&szID, 0);
      std::string::assign(&szID, arg2, strlen(arg2));
      LOBYTE(v38) = 1;
      CGameProcMain::MsgSend_ChatSelectTarget(*(CGameProcMain **)v31, &szID, CHAT_TARGET_SELECT_PRIVATE);
      LOBYTE(v38) = 0;
      std::string::_Tidy(&szID, 1);
      goto LABEL_74;
    case CMD_PRIVATE_TOWN:
      if ( CGameBase::s_pPlayer->_.m_bStun )
        goto LABEL_16;
      if ( 2 * CGameBase::s_pPlayer->_.m_InfoBase.iHP >= CGameBase::s_pPlayer->_.m_InfoBase.iHPMax
        || CGameBase::s_pPlayer->_.m_InfoExt.iZoneCur == 55 )
      {
        strcpy((char *)&data, "H");
        CAPISocket::Send(CGameProcedure::s_pSocket, (BYTE *)&data._allocator, 2);
      }
      else
      {
        data._allocator._byte = v30;
        std::string::_Tidy(&data, 0);
        LOBYTE(v38) = 2;
        CGameBase::GetText(IDS_ERR_GOTO_TOWN_OUT_OF_HP, &data);
        CGameProcMain::MsgOutput(*(CGameProcMain **)v31, &data, 0xFFFF00FF);
        LOBYTE(v38) = 0;
        std::string::_Tidy(&data, 1);
      }
      goto LABEL_74;
    case CMD_PRIVATE_EXIT:
      CGameProcMain::RequestExit(*(CGameProcMain **)v31);
      goto LABEL_74;
Getting back to the list itself, when it's opened it'll reset the selected list to the category list, and toggle the "option" button on CUICmd appropriately. When it sets the category, it naturally resets the content of the command list, and then depending on the selected (or default) category, it'll go through and fetch appropriately. For example:
        case CMD_GROUP_PRIVATE:
          v3 = 0;
          p_data_long = &CUICmdList::szCommandsPrivate[0]._data_long;
          do
          {
            CGameBase::GetText((e_TextResourceID)(v3 + 8100), &szMsg);
            v5 = *p_data_long;
            if ( !*p_data_long )
              v5 = NewFilename;
            data_long = szMsg._data_long;
            if ( !szMsg._data_long )
              data_long = NewFilename;
            sprintf(Buffer, data_long, v5);
            v7 = *p_data_long;
            if ( !*p_data_long )
              v7 = NewFilename;
            a2._allocator._byte = v56;
            std::string::_Tidy(&a2, 0);
            std::string::assign(&a2, v7, strlen(v7));
            v54 = 0xFF80FF80;
            v57 = (char **)&v53;
            LOBYTE(v75) = 1;
            sub_4A64F0(&v53, Buffer, &v56);
            v8 = v61->_.m_pList_Content;
            LOBYTE(v75) = 1;
            CN3UIList::AddString(v8, &a2, 0xFFC6C6FB, v53, v54);
            LOBYTE(v75) = 0;
            std::string::_Tidy(&a2, 1);
            p_data_long += 4;
            ++v3;
          }
          while ( (int)p_data_long < (int)&CUICmdList::ParseChattingCommand::szCmds[0][4] );
          break;
What they're (predominantly) doing here is they're just using the various command group enums (e.g. private's containing /PM, /TOWN, etc) and looping until they reach the end of this enum. For each of the categories.

Selecting a command predominantly just passes it through, but based on the selected category ID (and then consequently the appropriately selected command, by index in the array -- which is mapped to the previously mentioned enum), it'll determine whether or not it needs to show CUICmdEdit or not.

If it doesn't, it just passes it through to CUICmdList::ParseChattingCommand() to be handled.

CUICmdEdit's behaviour officially utilises CallBackProc to pass the notifications. On enter (specifically just DIK_RETURN, anyway), it'll forward the "1" event to its parent (CUICmdList) for handling.

CUICmdList then handles this in its CUICmdList::CallBackProc() implementation to fetch the command name and args, pass it to CUICmdList::ParseChattingCommand() and finally empties out the contents of CUICmdEdit's edit control.

I think that's about the general gist of how it works.

Thank you, @twostars, for the ASM code. I think I understand how it works now. Do you know how the tooltips is set for the commands in the official version? I tried using SetTooltipText perhaps I am missing some code for rendering it or something else.

twostars · 2024-05-23T19:52:38Z

They include the tooltip and the colour in CN3UIList::AddString(). I should note that they use 0xEFFFFFFF as the default tooltip colour to denote that it should use the default colour from the list itself, but otherwise they just handle it there.

xGuTeK · 2024-05-24T12:55:04Z

Oh i see AddString function has more parameters instead of one "szString"

CN3UIList::AddString(v8, &a2, 0xFFC6C6FB, v53, v54);

a2 = szString?
0xFFC6C6FB - color ?
v53 - tooltiptext?
v54 - tooltiptext color ?

so i will need also ASM for AddString function too

stevewgr · 2024-06-19T04:54:01Z

Oh i see AddString function has more parameters instead of one "szString"

CN3UIList::AddString(v8, &a2, 0xFFC6C6FB, v53, v54);

a2 = szString? 0xFFC6C6FB - color ? v53 - tooltiptext? v54 - tooltiptext color ?

so i will need also ASM for AddString function too

Yes, this is correct. Does this help you @xGuTeK?:

int CN3UIList::AddString(const CN3UIString & szString, D3DCOLOR crColor, std::string szToolTip, D3DCOLOR crToolTipColor)
{
  D3DCOLOR crFont;
  CN3UIString *v7; // eax
  CN3UIString *pString; // eax

  if ( crColor == 0xEFFFFFFF )
    crFont = this->CN3UIList.m_crFont;
  v7 = (CN3UIString *)operator new(0xECu);
  if ( v7 )
    pString = CN3UIString::ctor(v7);
  else
    pString = NULL;
  pString->vft->Init(pString, this);
  pString->vft->SetFont(
    pString,
    &this->CN3UIList.m_szFontName,
    this->CN3UIList.m_dwFontHeight,
    this->CN3UIList.m_bFontBold,
    this->CN3UIList.m_bFontItalic);
  pString->CN3UIString.m_Color = crFont;
  pString->vft->SetString(pString, szString);
  pString->CN3UIBase.m_szToolTip = szToolTip;
  pString->CN3UIBase.m_crToolTipColor = crToolTipColor;
  this->CN3UIList.m_ListString.push_back(pString);
  CN3UIList::UpdateChildRegions(this);
  return this->CN3UIList.m_ListString._Size - 1;
}

@twostars

…missing functions ![Screenshot](https://iili.io/d3T5Xn4.jpg) Thanks to @twostars and @stevewgr for asm code :)

Implement in game command list

de6d4af

twostars reviewed May 19, 2024

View reviewed changes

stevewgr marked this pull request as draft May 23, 2024 05:43

Update command list with working tooltips, add N3UIList, N3UITooltip …

526ec8e

…missing functions ![Screenshot](https://iili.io/d3T5Xn4.jpg) Thanks to @twostars and @stevewgr for asm code :)

xGuTeK force-pushed the Ui-CommandList1264 branch from ca0939e to 98c901d Compare July 9, 2024 21:23

stevewgr force-pushed the Ui-CommandList1264 branch 3 times, most recently from 7810241 to e254822 Compare July 22, 2024 03:09

stevewgr force-pushed the master branch from b1ab906 to 1d152cc Compare July 22, 2024 05:18

stevewgr force-pushed the Ui-CommandList1264 branch from e254822 to 526ec8e Compare August 3, 2024 06:35

stevewgr assigned xGuTeK and stevewgr Aug 9, 2024

xGuTeK added the official_client_code_review Needs verification against the official client code using IDA/Ghidra label Aug 27, 2024


		//////////////////////////////////////////////////////////////////////

		class CUICmdEditDlg : public CN3UIBase {


		//////////////////////////////////////////////////////////////////////

		class CUICmdListDlg : public CN3UIBase {

Uh oh!

Conversation

xGuTeK commented May 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

twostars May 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xGuTeK commented May 17, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars May 19, 2024 •

edited

Loading

twostars commented May 23, 2024 •

edited

Loading