axmolengine / axmol

Axmol Engine – A Multi-platform Engine for Desktop, XBOX (UWP) and Mobile games. (A fork of Cocos2d-x-4.0)
https://axmol.dev
MIT License
868 stars 195 forks source link

RichText URL link will not activate on click in certain conditions #1327

Closed rh101 closed 1 year ago

rh101 commented 1 year ago
auto visibleSize = _director->getVisibleSize();

auto* layout = ui::Layout::create();
layout->setContentSize(visibleSize);
layout->setAnchorPoint(Vec2::ANCHOR_MIDDLE);
layout->setPosition(Vec2(visibleSize.width / 2, visibleSize.height));
this->addChild(layout);

auto* listener = ax::EventListenerTouchOneByOne::create();
listener->setSwallowTouches(true);
listener->onTouchBegan = [](Touch*, Event*) -> bool { return true; };
_eventDispatcher->addEventListenerWithSceneGraphPriority(listener, layout);

auto* richText = ui::RichText::createWithXML("<a href='http://www.google.com'>click me</a>");
richText->ignoreContentAdaptWithSize(false);
richText->setContentSize(Size(500, 200));
richText->setPositionNormalized(Vec2(0.5f, 0.5f));

layout->addChild(richText);

If a RichText with a clickable URL is a in the scene, and there is another node in the scene that has a EventListenerTouchOneByOne listener, and the onTouchBegan returns true (along with setSwallowTouches(true)), then the URL handler will not be triggered.

The URL touch handler is implemented with this code (in UIRichText.cpp, class ListenerComponent):

_touchListener                 = ax::EventListenerTouchAllAtOnce::create();
_touchListener->onTouchesEnded = AX_CALLBACK_2(ListenerComponent::onTouchesEnded, this);
Director::getInstance()->getEventDispatcher()->addEventListenerWithSceneGraphPriority(_touchListener, _parent);

All EventListenerTouchAllAtOnce listeners will never be triggered if any EventListenerTouchOneByOne exist that returns true on onTouchBegan.

The reason is in function EventDispatcher::dispatchTouchEvent(EventTouch* event), where the dispatcher first calls all EventListenerTouchOneByOne listeners, and then if any return true for onTouchBegan, it removes the entries from a variable named mutableTouches (which holds all current touches).

Once it loops through all EventListenerTouchOneByOne listeners, it then gets to the following code which should handle all the EventListenerTouchAllAtOnce event listeners:

if (allAtOnceListeners && !mutableTouches.empty())
{
... handles all `EventListenerTouchAllAtOnce` here
}

The problem is that the if condition is false, since mutableTouches is empty, so it never enters that block, and no EventListenerTouchAllAtOnce listeners ever receive the touch inputs.

The main issue is that EventListenerTouchOneByOne and EventListenerTouchAllAtOnce are not sorted together (by priority and Z order), so even if a node using EventListenerTouchAllAtOnce is on top of every other node in the scene, it would never be called because of other nodes using EventListenerTouchOneByOne. This is what doesn't make sense to me.

My question is, does anyone know why EventListenerTouchAllAtOnce was used for the URL touch handler? If EventListenerTouchOneByOne is used, then this issue does not occur, but I'm not sure if there would be any side-effects.

All Widget sub-classes (like ui::Button etc) use EventListenerTouchOneByOne, and they all work as intended, even to call openURL.

One other thing, EventListenerTouchAllAtOnce is ONLY used for the RichText URL handler; I can't find any usage of it at all in the Axmol source code (not including the cpp_tests project of course).

rh101 commented 1 year ago

The way the current touch system works with regards to EventListenerTouchAllAtOnce and EventListenerTouchOneByOne is unexpected, and in my opinion, not correct. For example:

root_node (EventListenerTouchOneByOne)
├─ node_1 (EventListenerTouchOneByOne)
   ├─ node_2 (EventListenerTouchOneByOne)
      ├─ node_3 (EventListenerTouchAllAtOnce)

If you have a node tree as such, where node_3 is the one with the highest Z value, I would expect touch handling to be done in the order:

node_3, node_2, node_1, and finally root_node.

What actually happens at the moment is this order:

node_2, node_1, root_node, and finally node_3

From my understanding, touch handling should be done from the the highest Z order to the lowest. Splitting it into two different loops, one for EventListenerTouchAllAtOnce and another for EventListenerTouchOneByOne, means that the Z ordering between these two types of event listeners is not catered for.

I don't see any simple way to fix the EventDispatcher for this (without performance issues and such).

Now, regarding the RichText URL issue, I'll submit a PR to use EventListenerTouchOneByOne instead of EventListenerTouchAllAtOnce for the touch handling, and if anyone finds issues with it, then we'll figure out what to do next.