Changeset View
Changeset View
Standalone View
Standalone View
source/gui/CGUI.h
Show All 31 Lines | |||||
#include "ps/Shapes.h" | #include "ps/Shapes.h" | ||||
#include "ps/XML/Xeromyces.h" | #include "ps/XML/Xeromyces.h" | ||||
#include "scriptinterface/ScriptInterface.h" | #include "scriptinterface/ScriptInterface.h" | ||||
#include <boost/unordered_set.hpp> | #include <boost/unordered_set.hpp> | ||||
#include <map> | #include <map> | ||||
#include <vector> | #include <vector> | ||||
ERROR_TYPE(GUI, JSOpenFailed); | |||||
extern const double SELECT_DBLCLICK_RATE; | extern const double SELECT_DBLCLICK_RATE; | ||||
class CGUISpriteInstance; | class CGUISpriteInstance; | ||||
class CGUISprite; | class CGUISprite; | ||||
class IGUIObject; | class IGUIObject; | ||||
struct SGUIImageEffects; | struct SGUIImageEffects; | ||||
struct SGUIScrollBarStyle; | struct SGUIScrollBarStyle; | ||||
▲ Show 20 Lines • Show All 183 Lines • ▼ Show 20 Lines | public: | ||||
/** | /** | ||||
* Resolve the predefined color if it exists, otherwise throws an exception. | * Resolve the predefined color if it exists, otherwise throws an exception. | ||||
*/ | */ | ||||
const CGUIColor& GetPreDefinedColor(const CStr& name) const { return m_PreDefinedColors.at(name); } | const CGUIColor& GetPreDefinedColor(const CStr& name) const { return m_PreDefinedColors.at(name); } | ||||
shared_ptr<ScriptInterface> GetScriptInterface() { return m_ScriptInterface; }; | shared_ptr<ScriptInterface> GetScriptInterface() { return m_ScriptInterface; }; | ||||
JS::Value GetGlobalObject() { return m_ScriptInterface->GetGlobalObject(); }; | JS::Value GetGlobalObject() { return m_ScriptInterface->GetGlobalObject(); }; | ||||
/** | |||||
* Updates the object pointers, needs to be called each | |||||
* time an object has been added or removed. | |||||
* | |||||
* This function is atomic, meaning if it throws anything, it will | |||||
* have seen it through that nothing was ultimately changed. | |||||
* | |||||
* @throws PSERROR_GUI that is thrown from IGUIObject::AddToPointersMap(). | |||||
*/ | |||||
void UpdateObjects(); | |||||
private: | private: | ||||
/** | /** | ||||
* Adds an object to the GUI's object database | * Returns true if it succeeds to take over ownership of the object. | ||||
* Private, since you can only add objects through | |||||
* XML files. Why? Because it enables the GUI to | |||||
* be much more encapsulated and safe. | |||||
* | |||||
* @throws Rethrows PSERROR_GUI from IGUIObject::AddChild(). | |||||
*/ | */ | ||||
void AddObject(IGUIObject* pObject); | bool AddObject(IGUIObject& parent, IGUIObject& child); | ||||
elexis: Why this uses a reference for child:
1. **reference vs pointer:**
It's unexpected to take over… | |||||
/** | /** | ||||
* You input the name of the object type, and let's | * You input the name of the object type, and let's | ||||
* say you input "button", then it will construct a | * say you input "button", then it will construct a | ||||
* CGUIObjet* as a CButton. | * CGUIObjet* as a CButton. | ||||
* | * | ||||
* @param str Name of object type | * @param str Name of object type | ||||
* @return Newly constructed IGUIObject (but constructed as a subclass) | * @return Newly constructed IGUIObject (but constructed as a subclass) | ||||
▲ Show 20 Lines • Show All 381 Lines • Show Last 20 Lines |
Wildfire Games · Phabricator
Why this uses a reference for child:
It's unexpected to take over ownership of a pointer passed as a reference, however using a pointer would mean requiring error handling in case someone passes a nullpointer, while receiving a reference means that its not possible to receive a nullptr, which either results in a compiletime error or otherwise making it the obligation of the one passing the dereferenced pointer as reference at the time that happens. In that case the new and deref operator are performed in the same function, thus making it more obvious that no null-testing is needed and both less prone to null-deref errors and less prone to introducing writing useless (dead code), weird, buggy or unfinished exception handling.
That's why I chose the pointer for child despite that reference being used to take over ownership.
The alternative to pointer and reference are smart pointers, but that would also mean passing through their interface each std::map call.
However the derefence operation isnt slower for smart pointers, and allocation / erasure only happens once per GUI page (and not more often if assuming JS created GUI objects).
This way the deletion of GUI objects could also become the responsibility of the shared pointer, removing it from the CGUI destructor, removing the argument between deleting GUI object children in CGUI vs IGUIObject.
Then again that would be better off as a separate restructuring that doesnt fix a bug but changes from a nonbuggy state to a cleaner nonbuggy state. Then one will have more freedom to judge possible alternatives or more elaborate refactoring that can improve the code further and will add to auditability, bla. If there is no explicit use case for smart pointers, then it might even be easier to track where things are deleted (because smart pointers may be deleted in various places implicitly whereas the raw pointers are explicitly deleted, thus making it more evident if they are deleted in the wrong place or if they live longer than they should (leak).
For example if the object map of CGUI and the child object map in IGUIObject were shared pointers linking to the same object, there will be more cases to think about than when using raw pointers.