Windows native support using MSVC - Visual Studio 2017 #867

tusharpm · 2017-07-02T17:39:06Z

A step towards #9.
Fixes #508 with a modification - VS2017 instead of VS2015.
Supersedes #582 (credits to @YellowOnion).

## The procedure (this is here for my recollection later) - Get the dependencies using `vcpkg`. - Build/install the missing dependency `opusfile` manually. - Copy `dirent.h` from `vcpkg`'s downloads directory - it's named `fontconfig-dirent.h`. - Build using CMake - the usual workflow. - Install the `DejaVu Serif Book` font and copy the configuration to the fontconfig directory. - Modify the environment variables required to make the program run (hacks required before proper packaging).

A summary of the changes

A majority of the changes are for selectively compiling some code using preprocessor macros _MSC_VER or _WIN32. The former for MSVC specific code, and the latter if it is applicable to other compilers on Windows also.
Introduction of ~~LIBOPENAGE_API~~OAAPI for decorating the DLL's entry points for Windows. __declspec(dllexport) when compiling the DLL's sources, __declspec(dllimport) for the code including the header files only - the API user.
- Added an introduction in doc/code/pyinterface.md.
[Disrupting change] In libopenage/error/error.h, the variadic macro ENSURE has been changed to a two-argument macro. I left a TODO - kindly respond with the approach to choose here.
Python changes are mostly for path handling - the native path separator for Windows is a '\\' (weird, agreed).
CMake changes include path handling, finding dependent libraries and packages.
[Coordinate system hacks] The macros CONCAT and LOOP retain their variadic implementation, with the condition that they have a smaller arguments supported limit (3 for both). This should be superseded by the coordinate system overhaul in [WIP] Re-write of the coordinate system #662.
libopenage/error/stackanalyzer.cpp contains the skeleton code for Missing StackAnalyzer implementation for WIN32 #479. Without Missing util/compiler implementation for WIN32 #480 though, I don't expect it to provide a lot of useful information. The change includes a bug-fix in the GNU execinfo section.
Included ciso646 which provides the expansion for and, or, not, xor, etc.
Added a document describing the build instructions: doc/build_instructions/windows_msvc.md.

I've left a bunch of TODOs and HACKs around in the code. I can try to address the critical ones pointed out by the reviewers, the rest can be addressed subsequently.
I've built the project on Windows (using MSVC) only. I'll rely on the CI reports for other platforms (apologies for the cache thrashing). Please let me know if I need to run some test manually.

TheJJ

nice work :)

TheJJ · 2017-07-02T19:30:48Z

.gitignore

 /bin
 /.bin
+build
+deps


those should probably be /build and /deps, right?

I prefer disallowing such generic names throughout the repository (I found it useful for embedding thirdparty libraries). I can just make the change here.

TheJJ · 2017-07-02T19:40:10Z

buildsystem/check_py_file_list.py

    for filename in sorted(listed - actual):
        success = False
-        print("file was listed via add_py_module but does not exist: " +
+        print("file was listed via add_py_module but does not exist: " + filename + " as " +


are those changes relevant or just for your debugging purposes?

I put them in for my debugging. I left them there because I thought they'd be useful to others too.

TheJJ · 2017-07-02T19:42:54Z

buildsystem/cpp.cmake

 			endif()

-			if(NOT "${CMAKE_CXX_FLAGS}" MATCHES "-fsanitize")
+			if(NOT "${CMAKE_CXX_FLAGS}" MATCHES "-fsanitize" AND NOT MSVC)


so on windows undefined symbols in the libopenage would not throw errors, right? can we somehow display those errors with msvc?

I'm confused. Undefined symbols would result in link-time errors.
Can you please elaborate what errors we're looking for?

Undefined symbols are only discovered at link-time for executables, not for libraries like libopenage. Those cflags ensure that errors are shown nevertheless. I guess msvc has something that does the same.

Can you confirm or deny if libopenage is actually a shared library (possibly named libopenage.so or libopenage.dylib on other platforms)? If it is a shared library then the linker should complain about any missing symbols. AFAIK VS doesn't have an equivalent of -undefined dynamic_lookup, so no worries there. Otherwise, if it is a static library (like libopenage.a), I'll have to search in the MSVC docs for a flag which can do that, if one exists.

openage is and has to be a dynamic library so python can load it (import libopenage). This can never be done for a static library.

On Linux it did not complain, which is why we added those options. Hmm.

TheJJ · 2017-07-02T19:46:48Z

buildsystem/modules/FindPython.cmake

-	py_get_config_var(LIBDIR PYTHON_LIBRARY_DIR)
-	py_get_lib_name(PYTHON_LIBRARY_NAME)
+	if (PYTHON STREQUAL PYTHON_EXECUTABLE)
+		find_package(PythonLibs REQUIRED)


Where does PYTHON_EXECUTABLE come from?

From find_package(PythonInterp <other arguments>) around line 73. Afterwards from the cache.
I'll have to tweak some pieces of this file to make the buildsystem happy on all platforms. You can skip reviewing this file for now.

This solution is pretty elegant actually, if i understood it correctly :)
Could you please add some comment about the reason behind this? ("If the current python interpreter equals the one found at the very beginning with PythonInterp, we can use the PythonLibs find-module to find the matching libraries. otherwise we ask that interpreter where its matching libraries are", or something like that)

TheJJ · 2017-07-02T19:47:57Z

buildsystem/modules/FindPython.cmake

-		LINK_LIBRARIES "${PYTHON_LIBRARY_NAME}"
-		CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${PYTHON_INCLUDE_DIR}" "-DLINK_DIRECTORIES=${PYTHON_LIBRARY_DIR}" "-DCMAKE_CXX_STANDARD=14"
+		LINK_LIBRARIES ${PYTHON_LIBRARIES}
+		CMAKE_FLAGS "-DINCLUDE_DIRECTORIES=${PYTHON_INCLUDE_DIRS}" "-DCMAKE_CXX_STANDARD=14"


Don't we need the LINK_DIRECTORIES? If the library is in a non-standard path then the lib won't be found. I think that was a problem on mac then.

The variable PYTHON_LIBRARIES has the full path of the libraries (actually just one library per configuration) that need to be linked. I made (rather, tried to make) the variable consistent for other ways to locate the Python interpreter and libraries. I must've missed something that made kevin fail.

TheJJ · 2017-07-02T19:52:47Z

libopenage/util/compiler.h

+	#endif /* libopenage_EXPORTS */
+#else
+	#define LIBOPENAGE_API
+#endif


urghhhnngngngnnggnrhaargrgl. do we really need this pollution of preprocessor macros all over the codebase? what are the alternatives?

AFAIK, this is the only way to sanely* link to a DLL('s import library) with C++ interfaces in VS.
The alternatives while still using a shared library include:

Restricting to C-style interfaces (like extern "C" foo). Then using LoadLibrary and GetProcAddress (equivalents of dlopen and dlsym respectively) for using the DLL at runtime.

Go completely off-book and implement mangling for C++ identifiers (which is bound to break in the next release of VS).

Another alternative is to change libopenage to a static library - the problem would be it'll be linked to every python extension (.pyd) we build - bloating up the package when we build one.

Usually this macro doesn't scatter as much if the interface is small and consolidated. In our case, the interface with python is spread across the library.

*Sanity is relative. Essentially an alien concept to VS users.

Okay...

Is it common practice to name this ${LIBNAME}_API? Otherwise I'd suggest we use some "simpler" name, e.g. OAAPI, OAFUNC, or some idea of yours so it's less text pollution.

Apparently it isn't. I don't think there's a large enough convention to follow out there. I used what I knew from experience. I'm certainly open to using OAAPI as that strikes a nice balance between a ~~larg~~smaller and a descriptive name. OAFUNC, not so much, because this macro is used for classes and functions alike (actually even for externed objects, but I don't know what common name all of them can use). If you're okay with OAAPI, I can just do a find-replace all.

TheJJ · 2017-07-02T19:54:09Z

libopenage/util/fds.h

 	 * wraps an existing FD
 	 */
-	FD(int fd=STDOUT_FILENO);
+	FD(int fd);


What's the problem here? Does msvc have problems with default argument values?

Not really. Just with STDOUT_FILENO. Usually, it's defined in unistd.h which is just absent for VS. Turned out #define STDOUT_FILENO 1 was too much to ask for.

TheJJ · 2017-07-02T20:06:46Z

openage/convert/dataformat/data_definition.py

                        entry,
                        os.path.dirname(self.name_data_file)
-                    )
+                    ).replace(os.path.sep, '/') # HACK: Change to better path handling


no fear, this will totally be dropped with nyan.

VelorumS · 2017-07-03T17:35:02Z

libopenage/gui/guisys/private/gui_renderer_impl.h

 #include <mutex>
 #include <condition_variable>

+#include <epoxy/gl.h>


The GUI code must not depend on epoxy.

VelorumS · 2017-07-03T17:40:18Z

libopenage/gui/guisys/private/gui_subtree_impl.cpp

+#include "gui_renderer_impl.h"

 #include <cassert>
+#include <ciso646>


Never seen this header in the wild.

If it's about the and and or operators then it's better to replace them with the C++ operators.

http://en.cppreference.com/w/cpp/language/operator_alternative

Because in C++ these are built into the language, the C++ version of <iso646.h>, as well as <ciso646>, does not define anything.

-> no need to replace them with symbol operators and no need to include the header

http://en.cppreference.com/w/cpp/header/ciso646

... in a conforming implementation this header is empty.
It may still be required in order to have the alternative operator representations in old or non-conformant compilers.

And you would've guessed it already, MSVC is non-conforming at least in this case.

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

VelorumS · 2017-07-03T17:41:25Z

libopenage/gui/guisys/public/gui_renderer.h

-#else
-#include <OpenGL/gl.h>
-#endif
+#include <epoxy/gl.h>


The GUI code must not depend on epoxy.

Tomatower

Amazing work, just a few details!

Tomatower · 2017-07-03T21:40:29Z

libopenage/error/stackanalyzer.cpp

 	if (reversed) {
-		for (size_t idx = this->stack_addrs.size(); idx > 0; idx--) {
+		// `for (auto pc : this->stack_addrs | <ranges-ns>::view::reverse)` after Ranges-TS
+		for (size_t idx = this->stack_addrs.size(); idx-- > 0;) {


Why do you do the for like that?

The next line does a dereference using idx. I was initially planning to use a reverse iterator, but then got a hint from line 181 above to make a smaller change.

So you could leave the for as-is (for (size_t idx = this->stack_addrs.size(); idx > 0; idx--) {), and we wait util we have the reverse iterator?

Tomatower · 2017-07-03T21:41:48Z

libopenage/error/gl_debug.cpp


 namespace {
-void callback(GLenum source, GLenum, GLuint, GLenum, GLsizei, const GLchar *message, const void*) {
+void APIENTRY callback(GLenum source, GLenum, GLuint, GLenum, GLsizei, const GLchar *message, const void*) {


APIENTRY is not defined in a linux environment

It must be (I'm assuming) in epoxy/gl.h. I was relying on the CI reports to ensure that it is.

did not knew that. You are right that CI will see such things.

Tomatower · 2017-07-03T21:48:15Z

libopenage/shader/program.cpp


 #include "program.h"

-#include <unistd.h>


do we prefer to surround it with #ifndef _MSC_VER

I used that option only as a last-resort thing. The first (and IMO better) option is to identify and eliminate platform-dependent dependencies like this (technically unistd.h is part of the POSIX standard, but VS doesn't provide one). Same as above, if the CI is happy, we can remove this, right?

I agree with you, to eliminate such dependencies is best.

VelorumS · 2017-07-04T17:42:09Z

libopenage/error/error.h


 // ENSURE(condition, errormessage << variable << etcetc)
-#define ENSURE(...) do if (unlikely(not OPENAGE_ENS_FIRST(__VA_ARGS__))) throw ::openage::error::Error(MSG(err) OPENAGE_ENS_REST(__VA_ARGS__)); while (0)
+// TODO: Switch to variadic template if needed (or drop the variadic behaviour, if acceptable)


It may or may not be a macro.

Calling it with just one parameter (the 'condition') should produce a compiler warning.

VelorumS · 2017-07-04T18:05:15Z

libopenage/gui/guisys/public/gui_renderer.h

 #ifndef __APPLE__
+#ifdef _MSC_VER
+#define NOMINMAX
+#include <windows.h> // for WINGDIAPI, APIENTRY


The libopenage/gui/guisys deals only with Qt and SDL. It doesn't include anything else from openage or its dependencies like epoxy. The openage/libopenage/gui/integration (and openage/libopenage/gui/integration/private/gui_make_standalone_subtexture.h in it) is for connecting together Qt and openage, hence it uses epoxy that's used by openage textures.

This #ifdef boilerplate can be probably fixed by including some GL Qt header in gui_renderer.cpp file before the #include "gui_renderer.h". Qt basically does the same thing as epoxy.

VelorumS · 2017-07-05T06:04:25Z

libopenage/gui/guisys/public/gui_renderer.h

-#else
-#include <OpenGL/gl.h>
-#endif
+#include <QtGui/qopengl.h>


Can't really see the QtGui/qopengl.h in the Qt docs. It's probably more like a private header. I remember that I used #include <QOpenGLFunctions> somewhere instead.

The other thing is that the gui_renderer.h is a pimpl wrapper to isolate the openage code from Qt. So it can't include any Qt stuff. I can't try it right now, but that include should work in the gui_renderer.cpp.

A Qt examples page shows that it can be used directly [Sorry, they don't have line numbers; and I don't know the protocol for sharing their code without involving the copyrights/licences]. Given, I used a more explicit full path - which I can change to the shorter one if needed. OTOH, QOpenGLFunctions provides some wrappers which aren't required. I'm not well versed with Qt to take a decision either way.

I observed the pimpl idiom used in the gui code (here and in a few other places). For the inclusion of decided-upon file in gui_renderer.cpp:
Option 0: Before gui_renderer.h directly contradicts mom's recommendation. I'd prefer not to do that.
Option 1: After gui_renderer.h results in compilation failure for all units including it (error C3646: 'render': unknown override specifier and some others). ❌

Option (try larger change): Change/remove the GuiRenderer::render member to make it work with a forward-declared type GLuint. I'll need more information to make this work.
Option (none of these): Revert back to the expanded form before my last change.

Second best for me is the 'expanded form'.

TheJJ

We're getting there :)

It would be very good for other windows guys if you added some kind of introduction how the visualstudio project is generated so they can easily contribute. Maybe in doc/ide.md (or turn it into a folder so we have doc/ide/qtcreator.md and doc/ide/visualstudio.md)?

TheJJ · 2017-07-06T11:15:52Z

buildsystem/modules/FindPython.cmake

-	py_get_config_var(LIBDIR PYTHON_LIBRARY_DIR)
-	py_get_lib_name(PYTHON_LIBRARY_NAME)
+	if (PYTHON STREQUAL PYTHON_EXECUTABLE)
+		find_package(PythonLibs REQUIRED)


This solution is pretty elegant actually, if i understood it correctly :)
Could you please add some comment about the reason behind this? ("If the current python interpreter equals the one found at the very beginning with PythonInterp, we can use the PythonLibs find-module to find the matching libraries. otherwise we ask that interpreter where its matching libraries are", or something like that)

TheJJ · 2017-07-06T11:29:45Z

libopenage/error/stackanalyzer.cpp

 	if (reversed) {
-		for (size_t idx = this->stack_addrs.size(); idx > 0; idx--) {
+		// `for (auto pc : this->stack_addrs | <ranges-ns>::view::reverse)` after Ranges-TS
+		for (size_t idx = this->stack_addrs.size(); idx-- > 0;) {


So you could leave the for as-is (for (size_t idx = this->stack_addrs.size(); idx > 0; idx--) {), and we wait util we have the reverse iterator?

TheJJ · 2017-07-06T11:31:56Z

libopenage/error/stackanalyzer.cpp

+}
+
+// TODO: exact copy of the GNU execinfo counterpart below. Move to common place.
+void StackAnalyzer::get_symbols(std::function<void (const backtrace_symbol *)> cb,


do you want to address this now? it would just require some preprocessor adjustments? or should we do it later?

I made the changes.

TheJJ · 2017-07-06T11:42:25Z

libopenage/util/compiler.cpp

 std::string demangle(const char *symbol) {
+#ifdef _MSC_VER
+	// TODO: demangle names for MSVC;
+	return symbol;


Is there an msvc demangling abi function? if yes, can you please add it here?

There is: UnDecorateSymbolName. However, it has a problem mentioned in the Remarks section: it is single threaded. I can make this work with some synchronization primitives, but I'd prefer that to be in a subsequent PR, so the focus here remains on the build and we can get more opinions before going that way.
Suggestions?

TheJJ · 2017-07-06T11:43:24Z

libopenage/util/compiler.cpp


 std::string symbol_name(const void *addr, bool require_exact_addr, bool no_pure_addrs) {
+#ifdef _MSC_VER
+	return no_pure_addrs ? "" : addr_to_string(addr);


the symbol name resolution is missing, how much effort is it to add that?

Similar to above (although, clearly I forgot to add a TODO - I'll do that).
There's SymFromAddr, which has the same pitfalls as the one mentioned above.
This problem is actually discussed in #480, and I would like to know their opinions on using this approach.

TheJJ · 2017-07-06T11:44:11Z

libopenage/util/compiler.h

 #define unlikely(x)  __builtin_expect(!!(x), 0)
+#else
+#define likely(x)   (x)
+#define unlikely(x) (x)


doesn't msvc have branch preduction tuning?

I actually took it from a cython generated cpp file:

/* Test for GCC > 2.95 */ #if defined(__GNUC__) && (__GNUC__ > 2 || (__GNUC__ == 2 && (__GNUC_MINOR__ > 95))) #define likely(x) __builtin_expect(!!(x), 1) #define unlikely(x) __builtin_expect(!!(x), 0) #else /* !__GNUC__ or GCC < 2.95 */ #define likely(x) (x) #define unlikely(x) (x) #endif /* __GNUC__ */

I'm not aware of a branch prediction tuning feature for MSVC, if one exists.

TheJJ · 2017-07-06T11:55:03Z

libopenage/util/compiler.h

+	#endif /* libopenage_EXPORTS */
+#else
+	#define LIBOPENAGE_API
+#endif


Okay...

Is it common practice to name this ${LIBNAME}_API? Otherwise I'd suggest we use some "simpler" name, e.g. OAAPI, OAFUNC, or some idea of yours so it's less text pollution.

TheJJ · 2017-07-06T12:07:08Z

libopenage/util/compiler.h

+
+#if defined(_MSC_VER)
+#ifndef HAVE_SSIZE_T
+typedef int ssize_t;


Hm I think that is not correct, as int is not always the size of size_t.

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.htmlhttp://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html

The type ssize_t shall be capable of storing values at least in the range [-1, {SSIZE_MAX}].

Strictly speaking, we've used ssize_t wrong in some places in codebase, so we should fix that someday :)

On my machine SSIZE_MAX is defined as LONG_MAX, which is 2 ** 63 - 1, and bigger than int (2 ** 31 - 1).

I agree. And I'd like nothing more than a (POSIX?) standard file providing this declaration.
For instance, MSVC claims to provide SSIZE_T in BaseTsd.h. My problem with that, however, is with a disagreeing [re]definition from pyconfig.h. So I went with a non-conflicting definition, and wrapped it in _MSC_VER preprocessor guards. To be completely honest, I haven't even tried building this code for x64 yet. That would be another error to deal with.

I can check if we can reduce the usages of ssize_t. But, we would still need a definition that agrees with Python's headers, otherwise it could lead to other problems in the build.

Ah, the ssize_t is 32 bit wide from python because of your 32 bit platform i guess. So if we define it the way python does, it should be fine?

Python's headers make it bad to use as an exact copy:

/* _W64 is not defined for VC6 or eVC4 */ #ifndef _W64 #define _W64 #endif /* Define like size_t, omitting the "unsigned" */ #ifdef MS_WIN64 typedef __int64 ssize_t; #else typedef _W64 int ssize_t; #endif

Where MS_WIN64 is defined if another macro (_WIN64) is defined. I just made my definition work with the x86 compilation; For x64, I'll probably use the other definition and _WIN64 directly. Is that okay?

Hm, if there's no direct way, then so be it. But it seems strange that such definitions are necessary. Should be part of the standard library.

Tweak to find python library Fix inplacemodules for msvc build

Updates for MSVC build

Consolidate compiler specific attributes into macros Reduce/remove usage of unistd.h Define ssize_t appropriately for Windows Introduce `OAAPI` for decorating DLL entry points Reduce CONCAT and LOOP macros argument count

introduce `OAAPI`

TheJJ

looking very good now!

TheJJ added os: windows Windows-specific issue nice new thing ☺ A new feature that was not there before area: buildsystem Related to our cmake/python buildsystem lang: c++ Done in C++ code labels Jul 2, 2017

TheJJ requested changes Jul 2, 2017

View reviewed changes

TheJJ added the lang: python Done in Python code label Jul 2, 2017

VelorumS reviewed Jul 3, 2017

View reviewed changes

Tomatower reviewed Jul 3, 2017

View reviewed changes

tusharpm force-pushed the windows branch from 62cc3b5 to 40622a0 Compare July 4, 2017 14:44

Tomatower approved these changes Jul 4, 2017

View reviewed changes

VelorumS reviewed Jul 4, 2017

View reviewed changes

tusharpm force-pushed the windows branch 2 times, most recently from ef51e64 to f2f2af3 Compare July 4, 2017 20:28

VelorumS reviewed Jul 5, 2017

View reviewed changes

TheJJ requested changes Jul 6, 2017

View reviewed changes

tusharpm force-pushed the windows branch 4 times, most recently from 5df96d5 to 06773eb Compare July 9, 2017 04:03

tusharpm added 7 commits July 9, 2017 20:36

gitignore: Windows build files

53e6bb3

buildsystem: Fix paths and usage

102a823

Tweak to find python library Fix inplacemodules for msvc build

guisys: Use SFTtech#582 with modifications

fdc0066

Updates for MSVC build

libopenage: Fixes for MSVC

faae03b

Consolidate compiler specific attributes into macros Reduce/remove usage of unistd.h Define ssize_t appropriately for Windows Introduce `OAAPI` for decorating DLL entry points Reduce CONCAT and LOOP macros argument count

openage: HACK: path usage

2146930

doc: instructions for Windows MSVC

6b85e34

introduce `OAAPI`

Add name to copying.md

d7c4c40

tusharpm force-pushed the windows branch from 2cae06e to d7c4c40 Compare July 9, 2017 15:53

TheJJ approved these changes Jul 9, 2017

View reviewed changes

TheJJ merged commit 9661a69 into SFTtech:master Jul 14, 2017

TheJJ mentioned this pull request Jul 14, 2017

context_extraction win32 patch #582

Closed

tusharpm deleted the windows branch July 14, 2017 12:15

TheJJ mentioned this pull request Aug 13, 2017

[WIP]? Crosscompile for windows with MXE #609

Closed

Uh oh!

Windows native support using MSVC - Visual Studio 2017 #867

Windows native support using MSVC - Visual Studio 2017 #867

Uh oh!

Conversation

tusharpm commented Jul 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A summary of the changes

Uh oh!

TheJJ left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tusharpm Jul 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheJJ Jul 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Tomatower left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

tusharpm commented Jul 2, 2017 •

edited

Loading

tusharpm Jul 6, 2017 •

edited

Loading

TheJJ Jul 3, 2017 •

edited

Loading