Opened 4 months ago

Closed 6 weeks ago

#5442 closed defect (fixed)

Don't add GUI object type specific functions to JSInterface_IGUIObject

Reported by: elexis Owned by:
Priority: Should Have Milestone: Alpha 24
Component: Core engine Keywords:
Cc: Patch: Phab:D2136

Description

  • r22134 adds a getTextSize function to every GUI object type that works for 6 GUI object types and not for the other 6
  • Phab:D1781 proposes to add an addItem to every GUI object type that only works for one of the GUI object types and not for any other
  • Phab:D1751 proposes to add an method to every GUI object type that only works for one GUI object type and not for any other

But such GUI object type specific functions should be defined in a function specific to the affected object types, see the tirades in the references.

Change History (4)

comment:1 Changed 7 weeks ago by elexis

Milestone: BacklogAlpha 24
Patch: Phab:D2136

comment:2 Changed 6 weeks ago by elexis

In 22593:

Add virtual CreateJSObject to IGUIObject, split from GetJSObject, so that inheriting classes can extend the JS object upon construction, refs #5442, D2136.

Remove the comment from rP5154 that allures the reader to believe there is a memory leak which is not and has not been the case, refs D1700.
Remove the useless JSAutoRequest from GetJSObject if the object was constructed already (performance improvement), following cx assignment in rP14496, following JSAutoRequest addition in rP14877 (that also had removed other useles JSAutoRequest calls but not this one).
Don't change JS::PersistentRooted? to JS::Heap until SpiderMonkey is updated, refs D1700.
Provide access to GUI, ScriptHandlers? and JSObject for inheriting classes.

comment:3 Changed 6 weeks ago by elexis

In 22596:

Introduce a JSInterface_IGUITextOwner to encapsulate JSI_IGUIObject::getTextSize from rP22134 / D844.

JSI_IGUIObject should not contain functions that work only for some GUI Object types, refs #5442.

Deduplicate the shuffled copy of CText::SetupText?.

Avoid the 80 times slower dynamic_cast mandated by the virtual class inheritance by adding an ugly overridable pointer to the base class pointing at this derived classes, as bargained with Vladislav and proposed by wraitii in D1781 id=8426.
This may be improved by refactoring the IGUIObject and JSInterface classes to use templates and / or eliminating its virtual inheritance.

Implement and use FromJSVal / ToJSVal CSize specialization.
Remove the JS::CallArgsFromVp? call.

Differential Revision: https://code.wildfiregames.com/D2136
Comments By: wraitii, Vladislav

comment:4 Changed 6 weeks ago by elexis

Resolution: fixed
Status: newclosed

Fixed, since the other patches were not committed.

Note: See TracTickets for help on using tickets.