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

Barony next user data #86

Closed

Conversation

lheckemann
Copy link
Contributor

Same as #85 but for barony-next, I've resolved all the conflicts.

@addictgamer
Copy link
Collaborator

I tried merging, but I got errors:

/home/ciprian/programming/turning_wheel/barony_opensource/src/main.hpp:524:1: error: ‘light_t’ does not name a type

It looks like somebody changed the parameters lists for generatePolyModels and generateVBOs
Not sure what to do about the files.cpp conflict, inside of the saveMap function.

Also, shouldn't the header files have a header?

Also also, I noticed multiple violations of the coding style guidelines, such as:

void setUserDir(const char * const dir) {
//Curly brace not on proceeding line

and

if (mkdir(path, 0700) != 0 && errno != EEXIST)
		return errno;
//Lack of curly brace, and no inner padding on inside of outermost parentheses

@lheckemann
Copy link
Contributor Author

I'll take a look at it this weekend hopefully.

@addictgamer
Copy link
Collaborator

Ok.

I think I'm going to put off merging the PR until I can test it on a Mac again, when I get back home in August.

Since they are inlined, their implementation needs to be included in
every source file that uses them, otherwise it will go missing entirely
and fail to link.
cppfuncs.hpp:getLinesFromFile → files.cpp:getLinesFromDataFile

Also rewrite most of its implementation to provide useful errors
when the file cannot be opened.
- Add the openUserFile function, analogous to openDataFile, which opens
  files containing user data, i.e. config, saves and scores.
- Make the default location for user files $XDG_CONFIG_HOME/barony or
  $HOME/.config/barony if those variables are found. This should
  probably be replaced with behaviour more suitable for OSX
  ($HOME/Library/Application Support/barony?) and windows
  (%APPDATA%/barony?)
- Add -userdir= option which allows overriding the aforementioned
  behaviour
- Add setUserDir and setDataDir to the files module, so the knowledge of
  the static storage for these paths can be limited to files.cpp.
  Correspondingly,
- Remove public datadir variable
Move all openal/FMOD-specific functionality into
src/sound_{fmod,openal}.cpp. Now everything else should depend on the
common backend-agnostic interface provided in sound.hpp and the backend
is determined purely by linkage.
@lheckemann lheckemann force-pushed the barony-next-user-data branch from 227f30f to 806eafa Compare September 24, 2017 11:19
@lheckemann
Copy link
Contributor Author

Rebased, so it should merge cleanly now. I've also added some other niceties.

@lheckemann
Copy link
Contributor Author

Also, shouldn't the header files have a header?

What?

@addictgamer
Copy link
Collaborator

Also, shouldn't the header files have a header?
What?

E.g.:

/*-------------------------------------------------------------------------------

	BARONY
	File: stat.hpp
	Desc: header for stat.cpp (contains Stat class definition)

	Copyright 2013-2016 (c) Turning Wheel LLC, all rights reserved.
	See LICENSE for details.

-------------------------------------------------------------------------------*/

@lheckemann
Copy link
Contributor Author

Aah, right. Will add.

@lheckemann
Copy link
Contributor Author

Done!

@addictgamer
Copy link
Collaborator

Awesome. I'll pop open the Mac this weekend and do that last (hopefully) testing run, finally :)

@lheckemann
Copy link
Contributor Author

lheckemann commented Sep 26, 2017 via email

typo wasintroduced in f7d96cd
@lheckemann lheckemann mentioned this pull request Sep 27, 2017
@addictgamer
Copy link
Collaborator

Change the "Working Directory" to $(SolutionDir)$(Configuration)\

Ah, good catch.

For myself, I'd have to get it building for a start :p

Oh? What's the issue?

@lheckemann
Copy link
Contributor Author

Oh? What's the issue?

The usual when working OSX/Windows… not having a package manager to sort out all the dependencies for me.

This allows fixing sound_null to enable the game (although it will be
broken in a few ways, particularly when it comes to ranged weapons.
These should probably rely on time values rather than sound effects.).
@lheckemann lheckemann force-pushed the barony-next-user-data branch from cc8c83b to 662bed6 Compare October 18, 2017 16:58
@lheckemann
Copy link
Contributor Author

There, now this PR has a negative net lines-of-code balance and the game builds with sound_null :)

Still haven't built it on windows though.

@addictgamer
Copy link
Collaborator

addictgamer commented Oct 20, 2017

When building the editor (with OpenAL):

CMakeFiles/editor.dir/src/files.cpp.o: In function `loadMap(char const*, map_t*, list_t*)':
/home/ciprian/programming/turning_wheel/barony_opensource/src/files.cpp:647: undefined reference to `levelmusicplaying'

@addictgamer
Copy link
Collaborator

Error also occurs with FMOD, but works fine with sound null.

This allows the editor to link successfully without having
sound_common.cpp (which would introduce a dependency on net.cpp).
This results in a little duplication — setting levelmusicplaying = false
on every in-game map load — which is not ideal, but still IMHO better
than making loadMap care so much about where it's called from. I hope to
improve this more later on.
@lheckemann
Copy link
Contributor Author

That should fix it.

@lheckemann lheckemann mentioned this pull request Oct 20, 2017
8 tasks
@lheckemann
Copy link
Contributor Author

Any further issues?

@addictgamer
Copy link
Collaborator

I will review it this weekend.

@addictgamer
Copy link
Collaborator

FMOD and sound_null build now. OpenAL should probably be changed to the default sound library instead of FMOD.

Will text under Windows and Mac later this weekend.

@addictgamer
Copy link
Collaborator

Found an issue with FMOD:

When I start up the game and enter a new level, the level music doesn't start playing. Rather, the main menu's music drops to very low volume and keeps playing.

Then when I back out to the main menu, the main menus music keeps playing (very quietly), although barely slightly louder.

Then when I try to start a new game again, the main menu music starts blasting back at full volume.

@addictgamer
Copy link
Collaborator

addictgamer commented Nov 2, 2017

@lheckemann Have you had a chance to look into the FMOD issue?

@lheckemann
Copy link
Contributor Author

Just got back from a trip to Munich for a conference so I haven't had the time. I should get around to it this weekend though. Very strange problem though, not sure how far I'll get especially considering how cooperative FMOD has been in the past :/

@addictgamer
Copy link
Collaborator

addictgamer commented Nov 2, 2017

Ah, I see.

Thanks.

@lheckemann
Copy link
Contributor Author

This has become pretty much unworkable. I'll split it into smaller changes and PR them separately eventually.

@lheckemann lheckemann closed this Dec 19, 2017
@lheckemann lheckemann mentioned this pull request Dec 29, 2017
addictgamer pushed a commit that referenced this pull request May 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants