OpenXcom Forum
Contributions => Programming => Topic started by: grrussel on March 31, 2014, 10:03:19 pm
-
https://gist.github.com/grrussel/9899657
Fixes the OSX autobuilder.
L/usr/local/lib -L/usr/local/Cellar/yaml-cpp/0.5.1/lib -lSDLmain -lSDL -Wl,-framework,Cocoa -lyaml-cpp -lSDL_gfx -lSDL_mixer -lSDL_image -framework OpenGL -o ../bin/openxcom
Undefined symbols for architecture x86_64:
"OpenXcom::ComboBox::MAX_ITEMS", referenced from:
OpenXcom::ComboBox::setDropdown(int) in ComboBox.o
OpenXcom::ComboBox::setOptions(std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > > const&) in ComboBox.o
OpenXcom::ComboBox::setOptions(std::__1::vector<std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> >, std::__1::allocator<std::__1::basic_string<wchar_t, std::__1::char_traits<wchar_t>, std::__1::allocator<wchar_t> > > > const&) in ComboBox.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [../bin/openxcom] Error 1
-
I think this was already fixed here? https://github.com/SupSuper/OpenXcom/pull/800
-
Should be fixed in (not yet applied) pull https://github.com/SupSuper/OpenXcom/pull/803 (https://github.com/SupSuper/OpenXcom/pull/803)
-
Indeed, now fixed in the auto builder for OSX.
-
interesting -- what version of gcc were you compiling with on Linux?
-
Should be fixed in (not yet applied) pull https://github.com/SupSuper/OpenXcom/pull/803 (https://github.com/SupSuper/OpenXcom/pull/803)
This breaks on MSVC:
Error 1 error LNK2005: "private: static int const OpenXcom::ComboBox::LIST_MARGIN" (?LIST_MARGIN@ComboBox@OpenXcom@@0HB) already defined in State.obj
Error 2 error LNK2005: "private: static int const OpenXcom::ComboBox::MAX_ITEMS" (?MAX_ITEMS@ComboBox@OpenXcom@@0HB) already defined in State.obj
I'm not sure why a static int needs to be defined twice, since other classes (eg. Globe) do fine with just one definition. Maybe you need a clean?
-
if a clean doesn't work, you could also try declaring/defining like Globe::ROTATE_LONGITUDE
i.e.
in the .h:
class Globe
{
static const double ROTATE_LONGITUDE;
}
in the .cpp:
const double Globe::ROTATE_LONGITUDE = 0.10;
but yeah, clean should work
-
On Linux, where I got same error, I have gcc 4.7.3.
I'll try to clean next time to find out.
-
OSX builder is once again broken.
Looks like the C++ code is non-standard as written.
https://stackoverflow.com/questions/3025997/c-defining-static-const-integer-members-in-class-definition
-
Looks like MSVC have this as an extension, and offer a workaround for the multiple instances too
https://msdn.microsoft.com/en-us/library/34h23df8.aspx
Out of Class Definition of static const Integral (or enum) Members
Under the standard (/Za), you must make an out-of-class definition for data members, as shown here:
class CMyClass {
static const int max = 5;
int m_array[max];
}
...
const int CMyClass::max; https:// out of class definition
Under /Ze, the out-of-class definition is optional for static, const integral, and const enum data members. Only integrals and enums that are static and const can have initializers in a class; the initializing expression must be a const expression.
To avoid errors when an out-of-class definition is provided in a header file and the header file is included in multiple source files, use selectany. For example:
__declspec(selectany) const int CMyClass::max = 5;
-
OSX builder is once again broken.
Looks like the C++ code is non-standard as written.
https://stackoverflow.com/questions/3025997/c-defining-static-const-integer-members-in-class-definition
So why do all of these work? https://puu.sh/7TlZy.png
OpenXcom nightly builder also uses gcc and it compiles fine.
What's special about these statics? I'm really confused. ???
-
OK, I think I found correct answer. std::min require const int & (reference to const int). ComboBox::MAX_ITEMS only declared in header but not initialized, and ld linker cannot access to it. So most recommendations is initialize it outside of class (as we tried it before). Some linkers smart enough (or maybe bugged) to pass this situation.
To quick workaround I suggest to do this:
diff --git a/src/Interface/ComboBox.cpp b/src/Interface/ComboBox.cpp
index d90b08d..8124428 100644
--- a/src/Interface/ComboBox.cpp
+++ b/src/Interface/ComboBox.cpp
@@ -231,7 +231,9 @@ void ComboBox::setSelected(int sel)
*/
void ComboBox::setDropdown(int options)
{
- int items = std::min(options, MAX_ITEMS);
+ https:// Initialize const for std::min to avoid linking error on *nix
+ int max_items = MAX_ITEMS;
+ int items = std::min(options, max_items);
int h = _button->getFont()->getHeight() + _button->getFont()->getSpacing();
_window->setHeight(items * h + LIST_MARGIN * 2);
_list->setHeight(items * h);
-
FYI: the OSX builds are made with clang, not g++, so it makes for a 3rd possibility of compiler variation. But, essentially, declaring the statics as was done in the header only, and then taking the address of the static (as is done in calling std::min with its reference parameters) seems not to work on clang, some g++ versions, and is also not standard c++; As for the other instances of this, well, perhaps it is simply luck that they are not passed to std::min or a similar function or usage?
-
and it builds on OSX again :-)
-
As of change set:
ensure intro movie is centered on linux (detail)
Added multiline textlists. (detail)
The OSX build fails.
Interface/TextList.cpp:573:15: error: no matching function for call to 'min'
int selRow = std::min(_selRow, _rows.size());
^~~~~~~~
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/algorithm:2322:1: note: candidate template ignored: deduced conflicting types for parameter '_Tp' ('unsigned int' vs. 'unsigned long')
min(const _Tp& __a, const _Tp& __b)
^
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/../lib/c++/v1/algorithm:2314:1: note: candidate function template not viable: requires 3 arguments, but 2 were provided
min(const _Tp& __a, const _Tp& __b, _Compare __comp)
^
1 error generated.
-
diff --git a/src/Interface/TextList.cpp b/src/Interface/TextList.cpp
index 58adef3..f76de21 100644
--- a/src/Interface/TextList.cpp
+++ b/src/Interface/TextList.cpp
@@ -570,7 +570,7 @@ void TextList::setCondensed(bool condensed)
*/
int TextList::getSelectedRow() const
{
- int selRow = std::min(_selRow, _rows.size());
+ int selRow = std::min(_selRow, (unsigned)_rows.size());
return _rows[selRow];
}
fixes the compilation.
-
But size_t (https://www.cplusplus.com/reference/cstring/size_t/)'s are always unsigned. ???
-
i think it may be because it's comparing an unsigned int to an unsigned long, and can't match a <template>(T, T). casting the long to unsigned (that is, unsigned int) will make them the same bit size and match the template.
-
Not always 32bit in size, however ;-)
The OSX builds are 64bit, and therefore would have a 64bit size_t type
-
Good catch. I've replaced "unsigned int" with "size_t" where relevant.
-
One more;
Basescape/MiniBaseView.cpp:84:20: error: out-of-line definition of 'setSelectedBase' does not match any declaration in 'OpenXcom::MiniBaseView'
void MiniBaseView::setSelectedBase(unsigned int base)
^~~~~~~~~~~~~~~
diff --git a/src/Basescape/MiniBaseView.cpp b/src/Basescape/MiniBaseView.cpp
index 0dbbce3..2c797f4 100644
--- a/src/Basescape/MiniBaseView.cpp
+++ b/src/Basescape/MiniBaseView.cpp
@@ -81,7 +81,7 @@ size_t MiniBaseView::getHoveredBase() const
* the mini base view.
* @param base ID of base.
*/
-void MiniBaseView::setSelectedBase(unsigned int base)
+void MiniBaseView::setSelectedBase(size_t base)
{
_base = base;
_redraw = true;
-
Should be fixed.
-
@grrussel: check out https://github.com/SupSuper/OpenXcom/pulls?direction=desc&page=1&sort=created&state=open for submitting pull requests. it's easier on the devs than posting diffs in the forums, and the diffs will stay up to date even when surrounding code changes.
-
The makefile.simple no longer specifies a default target of OSX, so the OSX autobuilder got broken.
Now, the makefile can detect OSX automatically, se
https://gist.github.com/anonymous/626242249065b31f39d9
for the 3 line patch.
-
Added. I removed the OSX by default because some Linux users also use that Makefile and it threw them off :P