KnightOnline icon indicating copy to clipboard operation
KnightOnline copied to clipboard

drag/drop skills to HotkeyBar is broken/bugged

Open madpew opened this issue 8 years ago • 5 comments

It's not totally broken (I managed to get one skill to end up on the skillbar and usable).

Normally the skill icon just stays there (still attached to UISkillTree) and not being "received" by the Skillbar. Dragging/dropping items to the skillbar works fine (with some exceptions like the transformation gem).

madpew avatar Sep 11 '17 08:09 madpew

Huh, nice find. I don't think I've tried to drag/drop skills from the tree in years. Makes me wonder how long that would've gone unnoticed otherwise...

Not only are they not being attached, but they're not being reset on release either (which I believe should be a call to IconRestore()). Very interesting.

Edit: It's definitely registering the drop; so it's receiving it. It's not locking it into place / shifting it to the UI though, so hm.

twostars avatar Sep 11 '17 08:09 twostars

CUISkillTreeDlg isn't handling it like other things.

Ordinarily we go:

		case UIMSG_ICON_UP:
			// ¾ÆÀÌÄÜ ¸Å´ÏÀú À©µµ¿ìµéÀ» µ¹¾Æ ´Ù´Ï¸é¼­ °Ë»ç..
			if ( !CGameProcedure::s_pUIMgr->BroadcastIconDropMsg(CN3UIWndBase::m_sSelectedIconInfo.pItemSelect) )
				// ¾ÆÀÌÄÜ À§Ä¡ ¿ø·¡´ë·Î..
				IconRestore();				
			break;

And then handle behaviour appropriately in ReceiveIconDrop() (which is called by BroadcastIconDropMsg(). However, CUIHotKeyDlg only handles dragging items from the inventory, as you can see here:

	if ( CN3UIWndBase::m_sSelectedIconInfo.UIWndSelect.UIWnd != UIWND_INVENTORY )
		return false;

It would have to handle the instances specifically from the skill tree dialog, because they'll be skill icons not item icons (i.e. they won't have item data attached and the logic won't make sense).

What we do have, however is this:

		case UIMSG_ICON_UP:
			// Hot Key À©µµ¿ì¸¦ µ¹¾Æ ´Ù´Ï¸é¼­ °Ë»ç..
			{
				CUIHotKeyDlg* pDlg = CGameProcedure::s_pProcMain->m_pUIHotKeyDlg;
				if ( !IsIn(ptCur) && pDlg->IsIn(ptCur) )
				{
					if (!pDlg->IsSelectedSkillInRealIconArea())
					{
						// ¸®¼Ò½º Free..
						spSkill = CN3UIWndBase::m_sSkillSelectInfo.pSkillDoneInfo;
						if (spSkill)
						{
							// ¸Å´ÏÀú¿¡¼­ Á¦°Å..
							RemoveChild(spSkill->pUIIcon);

							// ¸®¼Ò½º Á¦°Å..
							spSkill->pUIIcon->Release();
							delete spSkill->pUIIcon;
							spSkill->pUIIcon = NULL;
							delete spSkill;
							spSkill = NULL;
						}
					}
				}
				else
				{
					// ¸®¼Ò½º Free..
					spSkill = CN3UIWndBase::m_sSkillSelectInfo.pSkillDoneInfo;
					if (spSkill)
					{
						// ¸Å´ÏÀú¿¡¼­ Á¦°Å..
						RemoveChild(spSkill->pUIIcon);

						// ¸®¼Ò½º Á¦°Å..
						spSkill->pUIIcon->Release();
						delete spSkill->pUIIcon;
						spSkill->pUIIcon = NULL;
						delete spSkill;
						spSkill = NULL;
					}
				}

				CN3UIWndBase::m_sSkillSelectInfo.pSkillDoneInfo = NULL;
				SetState(UI_STATE_COMMON_NONE);
				pDlg->CloseIconRegistry();
			}
			break;

So I think we more or less just need to scrap that, broadcast the message instead, and handle it appropriately. I think then things should hopefully come together.

twostars avatar Sep 11 '17 09:09 twostars

Just update the function CUI HotKeyDlg :: IsSelectedSkill RealIconArea ()

bool CUIHotKeyDlg::IsSelectedSkillInRealIconArea() { CN3UIArea* pArea; bool bFound = false; POINT ptCur = CGameProcedure::s_pLocalInput->MouseGetPos();

for( int i = 0; i < MAX_SKILL_IN_HOTKEY; i++ )
{
	pArea = CN3UIWndBase::GetChildAreaByiOrder(UI_AREA_TYPE_SKILL_HOTKEY, i);
	if (pArea && pArea->IsIn(ptCur.x, ptCur.y))
	{
		SetReceiveSelectedSkill(i); // add here
		bFound = true;
		break;
	}
}

if (!bFound) return false;
if (!CN3UIWndBase::m_sSkillSelectInfo.pSkillDoneInfo) return false;

return bFound;

}

ekremkraft avatar May 27 '20 07:05 ekremkraft

Unintentionally triggering behaviour (from the caller's perspective) in a method indicated as a check only is probably not the way to go there.

twostars avatar May 27 '20 20:05 twostars

Unintentionally triggering behaviour (from the caller's perspective) in a method indicated as a check only is probably not the way to go there.

I could not see a properly controlled method across the source code.

To explain;

Detects an icon drifting from "UISkillTreeDlg" here; CUISkillTreeDlg::ReceiveMessage case UIMSG_ICON_UP:

The icon is checked if it is on "UIHotKeyDlg" with "IsSelectedSkillInRealIconArea" method, but it does not fix the icon in any way.

The "CUIHotKeyDlg :: SetReceiveSelectedSkill" line I added above acts as "IconRestore" here and fixes the icon in the area.

There is no problem working properly. Could you please open some more what you mean? @twostars

ekremkraft avatar May 27 '20 20:05 ekremkraft