CParamNode::GetChild should return CParamNode& instead of CParamNode*

[Puel]

Ideally, GetChild? would return a CParamNode& instead of a CParamNode* (now that it will never return NULL), to simplify the usage syntax (GetChild?("x").GetChild?("y") instead of GetChild?("x")->GetChild?("y")).

[Puel]

I need some help here, since I don't feel confortable with the JavaScript interface yet.

---

../../../binaries/system/libsimulation2_dbg.a(ICmpTemplateManager.o): In function `void ScriptInterface_NativeMethodWrapper<CParamNode const&, ICmpTemplateManager>::call<std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >, CParamNode const& (ICmpTemplateManager::*)(std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >)>(JSContext*, int&, ICmpTemplateManager*, CParamNode const& (ICmpTemplateManager::*)(std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >), std::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >)': /home/puel/Desenvolvimento/Cpp/0ad/build/workspaces/gcc/../../../source/scriptinterface/NativeWrapperDefns.h:59: undefined reference to `int ScriptInterface::ToJSVal<CParamNode const&>(JSContext*, CParamNode const& const&)' collect2: ld returned 1 exit status make[1]: * binaries/system/pyrogenesis_dbg Error 1 make: * [pyrogenesis] Error 2 --- Anyone has any clue with this error msg?

[Philip]

Hmm, it looks like you're trying to change ICmpTemplateManager::GetTemplate to return a const CParamNode& instead of const CParamNode*. If so, then I think that's a case where it's better to continue returning a pointer, because that allows the function to signal errors (by returning NULL), and there aren't a lot of callers of that function so it's easy to make sure they all handle NULL returns correctly.

(Sorry I haven't done anything with #425 yet - been a bit busy, but I'll commit the changes in the next few days.)

[Puel]

I did what you said.

I also added a Ok flag in CParamNode. Preventing possible erros when copying a null CParamNode. (The old approach used the object's address to know if it is ok or not).

I was not unsure if it would be a acceptable perfomance payoff so there is a pre-processor macro - PARAMNODE_IGNORE_OK_FLAG - that when is defined, CParamNode will not have a OkFlag? saving one byte.

I think this macro is uneeded, but I was unsurely about the team's opinion.

[Philip]

Thanks! I'll apply this momentarily (with some minor changes - I tried to simplify the CParamNode interface by hiding the implementation details in the .cpp file and only exposing the necessary parts, and changed TemplateManager to fix a crash when running its tests, and cleaned up the CParamNode tests a bit).

The OK flag isn't a problem - we shouldn't have a lot of CParamNodes, and they're not very space-efficient anyway, so a few extra bytes won't hurt at all.

I should update the contributors list - do you have a name and/or email address that you would be happy to add there?

[philip]

(In [7302]) Make CParamNode not return NULL pointers, based on patch from Puel Fixes #425, #451

[Puel]

Hi, my name is André Puel. E-mail andrepuel at by the way dont spam me gmail.com

You will see more patches from me soonly... ;]

[Philip]

Added now. (I skipped the email address in that file (but kept a private record) since I don't want lots of obfuscated ones in there and it sounds like you don't want it publicised too widely :-) )

[anonymous]

Milestone Unclassified deleted