Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unicode file path support #156

Merged
merged 15 commits into from Jan 27, 2016
Merged

unicode file path support #156

merged 15 commits into from Jan 27, 2016

Conversation

Zyx-2000
Copy link
Contributor

I have finished unicode file support as discussed (http://forum.vcmi.eu/viewtopic.php?t=1135).

return nullptr;
}();

if (filename != nullptr && mode_fopen != nullptr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now mode_fopen can be nullptr? Did you forget ()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assign it nullptr if mode (the integer) has an invalid value. This is similar to how minizip does it internally; if an invalid mode is used, the FILE* will become a nullptr.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad - I misunderstood this contraption - we do not use lambdas this way, we define lambdas to call later.

#include <cstdio>


#ifdef _WIN32
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use #ifdef VCMI_WINDOWS

@alexvins
Copy link
Member

From travis log:

../lib/libvcmi.so: undefined reference to `typeinfo for boost::iostreams::stream<FileBuf, std::char_traits, std::allocator >'

../lib/libvcmi.so: undefined reference to `VTT for boost::iostreams::stream<FileBuf, std::char_traits, std::allocator >'

../lib/libvcmi.so: undefined reference to `FileStream::CreateFile(boost::filesystem::path const&)'

../lib/libvcmi.so: undefined reference to `FileStream::GetMinizipFilefunc()'

../lib/libvcmi.so: undefined reference to `FileBuf::close()'

../lib/libvcmi.so: undefined reference to `FileBuf::FileBuf(boost::filesystem::path const&, std::_Ios_Openmode)'

../lib/libvcmi.so: undefined reference to `FileBuf::seek(long, std::_Ios_Seekdir)'

../lib/libvcmi.so: undefined reference to `FileBuf::write(char const*, long)'

../lib/libvcmi.so: undefined reference to `FileBuf::read(char*, long)'

clang: error: linker command failed with exit code 1 (use -v to see invocation)

@Zyx-2000
Copy link
Contributor Author

FileStream.cpp was missing from the makefile, resulting in the above compilation error. I've now added it.

class DLL_LINKAGE FileBuf
{
public:
typedef char char_type;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong formatting in whole class, use TAB indent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't notice because it looks identical in code::blocks. Fixed.

@Zyx-2000
Copy link
Contributor Author

Is there anything else you would like me to do, to get this merged?

@alexvins
Copy link
Member

No, this need only testing.

@ArseniyShestakov
Copy link
Member

So I tested your branch by compiling it with MXE for Windows and results are:

  • Launcher works fine.
  • Unlike before changes client actually start with non-english username.
  • On attempt to start game server is crashing.
  • No log files are created.

Keep in mind that I tested it with username in russian.

@Zyx-2000
Copy link
Contributor Author

I had apparently overlooked implementing changes to the relevant logger class.

@alexvins
Copy link
Member

Launcher C::B project should be removed from workspace for now

@ArseniyShestakov
Copy link
Member

So logs works now, but I still can't server to start and it's looks like related to boost interprocess memory and permissions on Windows 8.1. So it's may be not even related to encodings, though removal of boost_interprocess doesn't help.

Is server working for you? What Windows version you test it with? Language?

@alexvins
Copy link
Member

pch still does not work for me, all related changes should be reverted.

@Zyx-2000
Copy link
Contributor Author

I'll remove the project file pch changes.

I can not reproduce the server crash. Running vista, language is swedish.

@alexvins
Copy link
Member

I also can`t reproduce server crash. On win7 russian (with Cyrillic username) it works (tested w|o launcher).

@ArseniyShestakov
Copy link
Member

So it's weird. I tried both Windows XP and Windows 7 64-bit and server crashing on startup of game (or refuse to start at all). Though as I said I have no idea if this related to encoding problems at all.

@alexvins alexvins self-assigned this Jan 27, 2016
@ArseniyShestakov
Copy link
Member

I tried @alexvins build and it's works just fine for me. So problem with server unrelated to your code.

@alexvins
Copy link
Member

No more changes here, I`ll test my c::b build one more time and merge with my project changes

@alexvins alexvins merged commit 92dd194 into vcmi:develop Jan 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants