Author Topic: Request: Refactor UIRect & subclasses  (Read 5463 times)

helmesjo

  • Full Member
  • ***
  • Thank You
  • -Given: 0
  • -Receive: 0
  • Posts: 116
    • View Profile
Request: Refactor UIRect & subclasses
« on: April 14, 2014, 02:08:41 AM »
NOTE: Sorry if I sound grumpy, just trying to explain how serious the issue is. ----->  :)

I know I've mentioned this before, but I have now come to the point where the careless design of UIRect is really getting in the way (requiring extensive modifications to add basic behaviour).

The main issue (from what I can tell) is that UIRect was designed last (instead of first) to "hack" in a common base class for UIWidget & UIPanel, but other than that, they share close to no behaviour. Pretty much all logic is delegated down to these subclasses, instead of handled in the base class. You have a base class UIRect, called UIRect since the common behaviour is a rectangle, but UIPanel & UIWidget is WAY different in their implementation. Sure, they do different things from your perspective, but for us clients using the API, there should be NO difference in sizing, positioning etc. since it's all just a rectangle in our eyes (ease of use is key here). The warning-bell should'v rang when you implemented the "SetRect"-method in UIWidget, but not in UIPanel... Just the name of that method should imply that it should be in UIRect, not UIWidget alone (nor delegated to them both from UIRect).
Another BIG issue is that this "design" destroys any way of caching values in a common way, since both UIWidget & UIPanel have their own way of representing the rectangle. So each time I want to use values in a common manner, I have to call "GetSides()", which recalculates everything based on each subclasses "rectangle-representation", destroying performance (or forces you to manually cache the values which is far to cumbersome).

The problems that arise are pretty minor when you only use your inspector-interface, since there it's just a few different input-fields. But when you try to add behaviour, it's frustrating to say the least. In my case, the poor implementation-design has forced me to heavily customise UIRect, UIWidget & UIPanel with code that should already be there to begin with. In my case, I've implemented a constraint-system which is really "simple" logically, but 75% of the implementation-process has been to bypass UIRects limitations.

Suggestion: Take some time to really think it through. Redo UIPanel & UIWidget to really SHARE the behaviour in a common way, not having their separate way of doing things. This will make things way easier for you and us (the users), since you will fulfil one of the most important principles: The Open/Closed Principle (http://en.wikipedia.org/wiki/Open/closed_principle). Not saying this will be easy, just saying it has to be done eventually (eventually being ASAP).

And as always, remember to have obvious events implemented (OnRectChanged, for example.).

Cherio! :)
« Last Edit: April 14, 2014, 03:31:31 AM by helmesjo »

ArenMook

  • Administrator
  • Hero Member
  • *****
  • Thank You
  • -Given: 337
  • -Receive: 1171
  • Posts: 22,128
  • Toronto, Canada
    • View Profile
Re: Request: Refactor UIRect & subclasses
« Reply #1 on: April 14, 2014, 03:33:40 AM »
I can make SetRect an abstract function in the UIRect easily enough, and you are correct in your guess that the issue lies in the fact that the rectangle came after everything else. It was my attempt to bring out common functionality into one class for the sake of anchoring things.

Add this function to UIPanel in the meantime:
  1.         public void SetRect (float x, float y, float width, float height)
  2.         {
  3.                 int finalWidth = Mathf.FloorToInt(width + 0.5f);
  4.                 int finalHeight = Mathf.FloorToInt(height + 0.5f);
  5.  
  6.                 finalWidth = ((finalWidth >> 1) << 1);
  7.                 finalHeight = ((finalHeight >> 1) << 1);
  8.  
  9.                 Transform t = cachedTransform;
  10.                 Vector3 pos = t.localPosition;
  11.                 pos.x = Mathf.Floor(x + 0.5f);
  12.                 pos.y = Mathf.Floor(y + 0.5f);
  13.  
  14.                 if (finalWidth < 2) finalWidth = 2;
  15.                 if (finalHeight < 2) finalHeight = 2;
  16.  
  17.                 baseClipRegion = new Vector4(pos.x, pos.y, finalWidth, finalHeight);
  18.  
  19.                 if (isAnchored)
  20.                 {
  21.                         t = t.parent;
  22.  
  23.                         if (leftAnchor.target) leftAnchor.SetHorizontal(t, x);
  24.                         if (rightAnchor.target) rightAnchor.SetHorizontal(t, x + width);
  25.                         if (bottomAnchor.target) bottomAnchor.SetVertical(t, y);
  26.                         if (topAnchor.target) topAnchor.SetVertical(t, y + height);
  27. #if UNITY_EDITOR
  28.                         NGUITools.SetDirty(this);
  29. #endif
  30.                 }
  31.         }
It's completely untested, but It Should Work™.

helmesjo

  • Full Member
  • ***
  • Thank You
  • -Given: 0
  • -Receive: 0
  • Posts: 116
    • View Profile
Re: Request: Refactor UIRect & subclasses
« Reply #2 on: April 14, 2014, 04:17:46 AM »
Thanks, but that's one of the things I've already implemented. But yeah, the point with this post was more to get confirmation that this is known and will be taken care of, since now I have about 20-30 min och merging every time I update NGUI (and fear of introducing hard-to-track bugs each time), and it would be nice to get out of that dark circle. :)

Thanks!

ArenMook

  • Administrator
  • Hero Member
  • *****
  • Thank You
  • -Given: 337
  • -Receive: 1171
  • Posts: 22,128
  • Toronto, Canada
    • View Profile
Re: Request: Refactor UIRect & subclasses
« Reply #3 on: April 15, 2014, 08:59:50 AM »
If you have Pro access, merging should be automatic.