Page MenuHomeWildfire Games

Various compilation fixes for different compilers
Needs ReviewPublic

Authored by Stan on Dec 17 2018, 2:47 PM.

Details

Reviewers
Itms
Trac Tickets
#4460
Summary

Add the fixes we don't have from here: https://github.com/rdb/fcollada/commit/f6186c5e9258106dbe82120a359c540876016f9e
Update the includes that didn't match the ones in the source folder.

Test Plan

/

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 6509
Build 10766: Vulcan BuildJenkins

Event Timeline

Stan created this revision.Dec 17 2018, 2:47 PM
Stan updated this revision to Diff 7051.Dec 17 2018, 2:56 PM

Add missing parenthesis

Vulcan added a subscriber: Vulcan.Dec 17 2018, 2:57 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/differential/825/

Stan changed the repository for this revision from rP 0 A.D. Public Repository to rFC FCollada.Dec 17 2018, 4:00 PM

Do we have to update our fcollada? Especially if it has useful changes.

If we have to, then my comments don't make sense. We just follow fcollada changes then.

include/FUtils/FUStringBuilder.h
139

Why uint64? The size_t isn't 64 bits always.

include/FUtils/FUTracker.h
119

I'd prefer to name a variable without leading underscores. This type of naming reserved for a global scope and for all scopes with a following capital letter.

122

This one and the same below fixes N6 from #5288.

include/FUtils/Platforms.h
128

No need to have a space after defined.

Stan added a comment.Feb 17 2019, 11:48 AM

Updating Fcollada is not that important. Nevertheless though our debug build is broken on windows because of it and the game doesn't work with Fcollada compiled with vs2015 instant default.

So I thought we might as well take all the fixes from the other forks over the internet.

Fcollada is not being maintained anymore. I believe we are the only ones maintaining it

In D1698#72233, @Stan wrote:

Updating Fcollada is not that important. Nevertheless though our debug build is broken on windows because of it and the game doesn't work with Fcollada compiled with vs2015 instant default.

So I thought we might as well take all the fixes from the other forks over the internet.

Ok. some of my comments make sense then :)

Could you attach the errors?

By the way, the game is compiled for me without the patch with VS2013. Probably we need to test the patch for VS2013 too until we drop its support.

Stan added a comment.Feb 17 2019, 1:24 PM

Errors for debug build are here https://trac.wildfiregames.com/ticket/4460
To reproduce just start a game in debug mode.

The Fcollada crash makes it impossible to package mods on windows.

For the crash after building Fcollada you just have to build it with vs2015. Vs 2013 works fine.

vladislavbelov updated the Trac tickets for this revision.Feb 17 2019, 1:28 PM
In D1698#72235, @Stan wrote:

Errors for debug build are here https://trac.wildfiregames.com/ticket/4460

Ok, it should be linked then (I added).

For the crash after building Fcollada you just have to build it with vs2015.
Vs 2013 works fine.

So you did a test of the patch on VS2013?

Stan added a comment.Feb 17 2019, 1:55 PM

I can't test on VS2013 but the goal is to drop support eventually and move on to 2015 2017 and maybe 2019

In D1698#72240, @Stan wrote:

the goal is to drop support eventually and move on to 2015 2017

It's true, but we didn't drop it yet.

and maybe 2019

Does the VS2019 support Windows XP?

Stan added a comment.Feb 17 2019, 3:37 PM

It's still using the 2017 toolset so yes.

In D1698#72244, @Stan wrote:

It's still using the 2017 toolset so yes.

Ok then. I'm waiting for answers of my comments. I agree with the patch in general.

Also it'd be good to test the patch on macOS and openBSD since the patches touches related macros.