VS 2012 Project and Solution Files

VS 2012 Project and Solution Files

Postby authenticate » 29 May 2013, 01:52

Hi everyone,

I spent some time getting your game to build and run down a native Windows tool chain.

You can find the forked repository on Github here (https://github.com/authenticate/ValyriaTear). In addition, I have a separate repository containing all the dependencies here (https://github.com/authenticate/Valyria ... pendencies).

If you want to try it out, here's what you need to do:
1.) git clone https://github.com/authenticate/ValyriaTear.git
2.) cd ValyriaTear
3.) git submodule update --recursive --init
4.) Open the VS 2012 solution file: .../ValyriaTear/vs2012/ValyriaTear.sln
5.) Build (F7)
6.) Run (F5)

I tested it on 64-bit Windows 8. So, if someone has problems on other versions of Windows, let me know. :P

It's not perfect yet. I have to clean up some of the dependencies, build a few more from source, and get the editor project online. However, I think it is in pretty good shape.

If you want to, I was hoping to get this merged into the main project. There's not really a whole lot of actual code changes required to get it to work. Just like 3 or 4 minor commits addressing iterators, locality, and a couple other small things (Well, this is if you ignore the commits about warnings. I hate warnings. I can always ease back the warning options in the IDE though. :P).

Thanks. And nice game btw.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 31 May 2013, 11:12

Hi authenticate,

Thank a lot for your interest in Valyria Tear. :D
If you want to, I was hoping to get this merged into the main project. There's not really a whole lot of actual code changes required to get it to work. Just like 3 or 4 minor commits addressing iterators, locality, and a couple other small things (Well, this is if you ignore the commits about warnings. I hate warnings. I can always ease back the warning options in the IDE though. :P)


You can freely have a go at the commit preparing the iterators for VS2012. :) We'll see what's left afterwards.

Best regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 01 Jun 2013, 22:31

Sounds good.

Here's the proposed changes to address the iterator assertion.

In src\modes\map\map_events.cpp around line 1299,

...
void EventSupervisor::Update()
{
// Update all launch event timers and start all events whose timers have finished.
for (size_t i = 0; i < _active_delayed_events.size(); ++i) {

// Retrieve the pair from the vector.
std::pair<int32, MapEvent*> *pair = &(_active_delayed_events[i]);

// Update its time.
pair->first -= SystemManager->GetUpdateTime();

if (pair->first <= 0) { // Timer has expired

// Store the event.
MapEvent *start_event = pair->second;

// Remove it from the vector.
_active_delayed_events.erase(_active_delayed_events.begin() + i);

// We begin the event only after it has been removed from the launch list
StartEvent(start_event);
} else {

++i;
}
}
...

The forum is probably going to format the indenting all weird. Hopefully, it's somewhat readable.

In the original version, the debug run time asserts here:

it != _active_delayed_events.end();) {

because of invalid iterators. Now, to be honest, I don't know why it would assert. The following line

it = _active_delayed_events.erase(it);

should be fine since 'erase' returns an updated iterator. In addition, the loop right below this one does something very similar and it doesn't assert. So, I don't really know what's going on. However, I just changed the loop to use indices to get things working.

Let me know what you think.

Thanks.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 02 Jun 2013, 16:34

Hi Authenticate, :)

Remember you can use the code tag for such examples.

{l Code}: {l Select All Code}
{
    it = can_be(useful);
}


I see the point but honestly I don't like the change, since it is abusing iterators on a map which looks weird, and this way of coding shouldn't trigger an error anyway, since the same technique is used in many other places in the code, just as you pointed.

That said, you might have discovered a bug and it might be good to take the time to understand what's going on before hiding it. :)

So, could you give me the error here so I can have a better view of what is happening?

Best regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 03 Jun 2013, 01:59

Hi,

Yeah. I agree.

I dug a little deeper to try and figure out what exactly is happening. Let me put some code up here as a reference:

{l Code}: {l Select All Code}
void EventSupervisor::Update()
{
...
        if(it->first <= 0) {  // Timer has expired
            MapEvent *start_event = it->second;
            it = _active_delayed_events.erase(it);
            // We begin the event only after it has been removed from the launch list
            StartEvent(start_event);
        }
...
}


{l Code}: {l Select All Code}
void EventSupervisor::_ExamineEventLinks(MapEvent *parent_event, bool event_start)
{
...
        // Case 3: The child event has a timer associated with it and needs to be placed in the event launch container
        else {
            MapEvent *child = GetEvent(link.child_event_id);
            if(child == NULL) {
                IF_PRINT_WARNING(MAP_DEBUG) << "can not launch child event, no event with this ID existed: "
                                            << link.child_event_id << std::endl;
                continue;
            } else {
                _active_delayed_events.push_back(std::make_pair(static_cast<int32>(link.launch_timer), child));
            }
        }
...
}


So, during the iteration over all active, delayed events, if a delayed event is ready to become active, we call start event which puts the delayed event into the active events vector. In addition, the start event function eventually calls into the examine event links function. Now, in case 3, when we need to add a delayed event, we call "push_back" which is what causes the problem and makes this loop different than all the others. We are currently in the middle of iterating over the active delayed events. Adding another event to it now invalidates our current iterator, "it", because push back potentially could cause a reallocation of the entire vector.

I get this assert during the intro scene right before the scene darkens and the lightning strikes. So, in this particular case, it doesn't actually reallocate. So, there won't actually be a run time failure due to an invalid iterator. And to be honest, since we delete an element right before we start calling down to where we call push back, it should never actually reallocate anyways. (However, if you had an event which spawned a lot of delayed events, you might be able to get it to crash this way.)

This makes some sense too.

{l Code}: {l Select All Code}
it = _active_delayed_events.erase(it);
// We begin the event only after it has been removed from the launch list
StartEvent(start_event);


The comment here initially seemed odd to me. Why do I have to remove the event first? It doesn't seem like it should really matter. The event looks to be allocated on the heap. Seems harmless?

However, the comment is absolutely correct. If you reorder those lines of code, the run time not only asserts, but it will promptly crash, even in an optimized build.

So, the next logical question is why. Why does it crash? That iterator hasn't changed yet. We are pushing to the back not the front.

So, I don't know what's going on for certain. However, if I had to speculate, calling start event first means we are calling push back first. The probability of causing a vector reallocation in this case is actually very, very high since we haven't freed up a slot in our vector yet. Now, if we assume it did reallocate, the following erase call will be operating on memory that has already been freed by the vector, which is probably what causes the crash.

So, hopefully, that made some amount of sense.

The last question I guess is what do you want to do about it? :)

In my personal, very biased, opinion, I like iterators; I don't like indexing. However, there's a time and place for everything. Iterators lose a lot of their power if you have an algorithm that adds and removes items during the loop. The more I look at this problem, I actually kind of like the indexed solution. Indexing even solves the chronological dependency on erasing and then starting an event. Do it either way with indexing. The order no longer matters.

On the other hand, something like this could work:

{l Code}: {l Select All Code}
it = _active_delayed_events.erase(it);
it = StartEvent(it);


However, I haven't actually looked into implementing start event like this in a way that would fix the iterator problem. You would probably need some ugly code to update an iterator as a return value.

Anyways, let me know what you think.

Thanks.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 03 Jun 2013, 15:00

Hi authenticate, :)

Thanks a lot for the detailed answer as it permitted me to clearly get the design bug here. :D

As for me, the delayed events that are becoming active should be triggered only after parsing the _active_delayed_events loop.
Here is the change I would propose. Does it fix the issue for you?

{l Code}: {l Select All Code}
    // will store the events that became active in the delayed event loop.
    std::vector<MapEvent *> events_to_start;

    // Update all launch event timers and start all events whose timers have finished
    for(std::vector<std::pair<int32, MapEvent *> >::iterator it = _active_delayed_events.begin();
            it != _active_delayed_events.end();) {
        it->first -= SystemManager->GetUpdateTime();

        if(it->first <= 0) {  // Timer has expired
            MapEvent *start_event = it->second;
            it = _active_delayed_events.erase(it);

            // We add the event ready to start in a vector, waiting for the loop to end
            // before actually starting it.
            events_to_start.push_back(start_event);

        } else {
            ++it;
        }
    }

    // Starts the events that became active.
    for(std::vector<MapEvent *>::iterator it = events_to_start.begin(); it != events_to_start.end(); ++it) {
        StartEvent(*it);
    }


Best regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 04 Jun 2013, 01:40

No problem.

I just fetched the changes and rebased to the origin.

This part looks good and runs fine now. Good job.


The next issue falls into the realm of locality. Here's an excerpt from ...\src\engine\system.cpp:

{l Code}: {l Select All Code}
void SystemEngine::SetLanguage(const std::string& lang)
{
    Reinitl10n();

    _language = lang;
    setlocale(LC_MESSAGES, _language.c_str());
    setlocale(LC_ALL, "");

#ifdef _WIN32
    std::string lang_var = "LANGUAGE=" + _language;
    putenv(lang_var.c_str());
    SetEnvironmentVariable("LANGUAGE", _language.c_str());
    SetEnvironmentVariable("LANG", _language.c_str());
#else
    setenv("LANGUAGE", _language.c_str(), 1);
    setenv("LANG", _language.c_str(), 1);
#endif
}


The problem is in this line:

{l Code}: {l Select All Code}
setlocale(LC_MESSAGES, _language.c_str());


The native Windows headers don't support this. Here is part of the native Windows locale.h:

{l Code}: {l Select All Code}
/* Locale categories */

#define LC_ALL          0
#define LC_COLLATE      1
#define LC_CTYPE        2
#define LC_MONETARY     3
#define LC_NUMERIC      4
#define LC_TIME         5

#define LC_MIN          LC_ALL
#define LC_MAX          LC_TIME


I've only ever seen locality done on Windows using environment variables which is already present in the #ifdef part of that function. So, to get things working on my end, I just changed the function to this:

{l Code}: {l Select All Code}
void SystemEngine::SetLanguage(const std::string& lang)
{
    Reinitl10n();

    _language = lang;

#ifdef _WIN32
    setlocale(LC_ALL, "");
    std::string lang_var = "LANGUAGE=" + _language;
    putenv(lang_var.c_str());
    SetEnvironmentVariable("LANGUAGE", _language.c_str());
    SetEnvironmentVariable("LANG", _language.c_str());
#else
    setlocale(LC_MESSAGES, _language.c_str());
    setlocale(LC_ALL, "");
    setenv("LANGUAGE", _language.c_str(), 1);
    setenv("LANG", _language.c_str(), 1);
#endif
}


Now, I guess the question is, is this ok? Does it work? On Visual Studio it does. ;)

In addition, the functionality for Linux shouldn't change either. However, for people cross compiling using MinGW for Windows, I don't really know. Maybe it was like that for a reason? Maybe it wasn't? I don't know.

Have a good one.

Thanks.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 04 Jun 2013, 08:45

Hi authenticate, :)

LC_MESSAGES is coming from gettext and should normally be a copy of LC_ALL on Windows. But it seems it is not in your case.

The current code is compiling fine using MingW but I guess this is coming from the fact they're defining LC_MESSAGES to be equal
to LC_ALL there.

Would adding something like this be ok, btw?

{l Code}: {l Select All Code}
// define the LC_MESSAGES if it isn't already done
#ifndef LC_MESSAGES
#define LC_MESSAGES LC_ALL
#endif


Best regards, :)
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 05 Jun 2013, 02:02

I investigated some of my dependencies and that's correct.

In my version of libintl.h, there's the following:

{l Code}: {l Select All Code}
#include <locale.h>

/* The LC_MESSAGES locale category is the category used by the functions
   gettext() and dgettext().  It is specified in POSIX, but not in ANSI C.
   On systems that don't define it, use an arbitrary value instead.
   On Solaris, <locale.h> defines __LOCALE_H (or _LOCALE_H in Solaris 2.5)
   then includes <libintl.h> (i.e. this file!) and then only defines
   LC_MESSAGES.  To avoid a redefinition warning, don't define LC_MESSAGES
   in this case.  */
#if !defined LC_MESSAGES && !(defined __LOCALE_H || (defined _LOCALE_H && defined __sun))
# define LC_MESSAGES 1729
#endif


Now, I have no idea why it's 1729. Maybe I configured the library incorrectly when I built it?

I just changed to it to

{l Code}: {l Select All Code}
# define LC_MESSAGES LC_ALL


like you suggested and everything is fine now. So, I wouldn't worry about doing anything to your code base about it. :)


We're down to one last issue. It occurs in two places:

../src/common/global/global_objects.cpp
{l Code}: {l Select All Code}
_weapon_animations[char_id].insert(std::make_pair<std::string, std::string>(anim_alias, anim_file));


../src/engine/video/image.cpp
{l Code}: {l Select All Code}
frames_offsets.push_back(std::make_pair<float, float>(x_offset, y_offset));


These calls each return the following error respectively:

'std::make_pair' : cannot convert parameter 1 from 'std::string' to 'std::string &&'
'std::make_pair' : cannot convert parameter 1 from 'float' to 'float &&'

These errors result from the VS 2012 compiler's adoption of some of the new C++11 features. It demands to use the new RValue reference optimizations.

The fix on my end is pretty easy. You can just drop the template specifications and let the compiler sort it out:

{l Code}: {l Select All Code}
_weapon_animations[char_id].insert(std::make_pair(anim_alias, anim_file));
frames_offsets.push_back(std::make_pair(x_offset, y_offset));


Now, I've never tested this, but g++ might require similar adjustments if you configured it to compile with the new C++11 features flag.

Anyways, let me know what you think.
Have a good one.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 05 Jun 2013, 08:05

Hi authenticate,

Now, I have no idea why it's 1729. Maybe I configured the library incorrectly when I built it?

My wild guess would be that other compilers don't strictly check whether this value is higher than LC_MAX / LC_TIME, but it's just my opinion. :)

Now, I've never tested this, but g++ might require similar adjustments if you configured it to compile with the new C++11 features flag.

That's the core point, I'd like to avoid C++YYx options, as it's only narrowing the platforms/OS/distro versions supported for absolutely no gain at the moment.
I do think this project doesn't need it anyway. Once the c++11x option will be made the default one on every open sourced compilers, we'll talk again about it. ;)

Is it possible for you to remove the c++11x support flag when compiling and try again?

Best regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 06 Jun 2013, 02:23

No. It's not possible. :(

The Microsoft tools do not support those kind of compiler flags. If you use their new compiler and tool chain, you get the new language features. I guess they figure that their employees took the time to build it. So, you have to use it. :D

Now, don't get me wrong, I agree with you on the compatibility with features. I am not going sit and argue about adopting lambda functions or whatever else. At the end of the day, it just doesn't really matter. :) There's no real need.

However, I don't think this is one of those cases. There's just a couple lines of extra verbosity in the code base that, if removed, everyone's happy. G++ is happy; Clang is happy; VC++ is happy. Every one wins. :cool:

In all seriousness though, I don't think it would break anything. Older and newer compilers alike should be able to resolve the templates without explicitly specifying the template types in these cases.

Have a nice day.

Thanks.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 06 Jun 2013, 08:57

Hi authenticate, :)

Now, don't get me wrong, I agree with you on the compatibility with features. I am not going sit and argue about adopting lambda functions or whatever else. At the end of the day, it just doesn't really matter. :) There's no real need.


Eh eh. I guess many of us had this discussion many times. ;) Now I see your point better.

I've just tested your change and it's working fine, so they will be pushed as soon as I have access to all that. (Or maybe you want to make merge request?)

Thanks for the help. I really appreciate it!

Best regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 07 Jun 2013, 04:34

No problem.

Thanks for adding the changes. Everything is working now. :)

Sorry about not doing the merge requests for all these things. I'm still pretty new to git and github. I didn't fully understand what all was going on at first. They really did a nice job at improving how to manage open source projects. It's actually pretty amazing. It's super easy to fork and work on projects and then push change sets back to the original project. And even get updates from the original project. That's crazy. Project management used to be a huge problem and required someone to spend a lot of time organizing it all.

Have a good one.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 07 Jun 2013, 07:58

Hi :)

Cool to see it's coming along!

This made me think about one question:
How did you get to know about Valyria Tear, btw? (Just for my personal curiosity, I must say ;])

Also, what is left? Do you want to merge the vs project files? Is it possible to do that in a folder? (Alla XCode)?

Best Regards,
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 08 Jun 2013, 20:20

Bertram {l Wrote}:How did you get to know about Valyria Tear, btw?


I was googling around for open source RPG's. I originally found the Hero of Allacrost project. However, it didn't seem like that project was very active anymore. I did some more searching and eventually found this project primarily because the code base is a continuation of Hero of Allacrost.

Bertram {l Wrote}:Also, what is left? Do you want to merge the vs project files? Is it possible to do that in a folder? (Alla XCode)?


Yeah. That would probably be the best thing to do. Right now, it's setup like

...\PathToRepo\ValyriaTear\vs2012\

All the VS project and solution files are down in there and they're configured to read everything from the project root's directory.

I still need to fix up some things related to lua before I would recommend merging it in. This was the first time I've ever integrated lua and luabind into a C++ project. Originally, I was getting a lot of run time warnings related to both luabind and lua. I went back and reconfigured and rebuilt luabind to fix it. I still need to do the same thing for the lua library itself.

It's not a huge problem. It still works correctly. However, it's really annoying. It spams the console window way too much.

Have a good one.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 09 Jun 2013, 21:19

Hi :)

Good to know!

I still need to fix up some things related to lua before I would recommend merging it in. This was the first time I've ever integrated lua and luabind into a C++ project. Originally, I was getting a lot of run time warnings related to both luabind and lua. I went back and reconfigured and rebuilt luabind to fix it. I still need to do the same thing for the lua library itself.

It's not a huge problem. It still works correctly. However, it's really annoying. It spams the console window way too much.

Do you mean there is some warning killing to do inside the luabind/ project folder as well? Or just compiler options to set up?

Best regards, :)
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Re: VS 2012 Project and Solution Files

Postby authenticate » 10 Jun 2013, 03:18

I just added a preprocessor define to the luabind project:

LUABIND_NO_ERROR_CHECKING

and this quieted everything. I didn't change anything with the code itself.

Thanks.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby authenticate » 12 Jun 2013, 03:42

Hi,

Thanks for merging the VS project and solution files.

Here's some information to put into the read me about the dependencies.


To build on Windows with Visual Studio 2012:

1.) git clone https://github.com/authenticate/Valyria ... encies.git
2.) Copy the ValyriaTear-VS2012-Dependencies folder into your Valyria Tear repository.
3.) Open the VS 2012 solution file: .../ValyriaTearRepository/vs2012/ValyriaTear.sln
4.) Build (F7)
5.) Run (F5)


That should let people use it. :)

Thanks.
authenticate
 
Posts: 10
Joined: 29 May 2013, 01:20

Re: VS 2012 Project and Solution Files

Postby Bertram » 12 Jun 2013, 11:31

Hi,

It is done! thanks a lot! :)
User avatar
Bertram
VT Moderator
 
Posts: 1652
Joined: 09 Nov 2012, 12:26

Who is online

Users browsing this forum: No registered users and 1 guest

cron