Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old April 9, 2016, 14:16   #101
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 54
Posts: 7,645
Donated: $60
Nick is on a distinguished road
Quote:
Originally Posted by calris View Post
As for the Linux situation, can you tell me what the ./configure options are that result in register_sound_pref_parser not being defined?
I think I understand it better now, and it's not really related to the configure options, it's to do with where sound-core.c is linked.

Currently the build system compiles most of the files, links them to form angband.o, then compiles the main files and links them to form the final binary angband. You have set it up so that sound-core is compiled and linked in that second step, but the unit testing suite only uses angband.o. So when the linker looks at angband.o preparatory to compiling the test suite and linking them, it can't find register_sound_parser_pref(), because it isn't in angband.o.

The obvious quick fix I attempted was to compile sound-core into angband.o, but that results in tests having problems with init_sound_sdl not being defined...

I'm not sure what the best solution is, but I'll give it some more thought - if you have a good answer, I'd be delighted to hear it
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old April 9, 2016, 15:26   #102
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
If SOUND is not defined and sound.h is included (which it is via ui-prefs.h), then the stub version of register_sound_pref_parser() will be included in angband.o

If SOUND is defined, then the platform specific code needs to define register_sound_pref_parser()

The obvious solution is to make sure SOUND is not defined when unit tests are build (since, as you say, no platform specific code is compiled for the unit tests)

I'm assuming the 'unit tests' define a special 'platform specific main' (the platform being the unit test suite). So of course, we could always define some kind of dummy sound module for the unit tests (which would be rather useful for testing the parsing of the sound preferences file)
calris is offline   Reply With Quote
Old April 9, 2016, 16:21   #103
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,923
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by calris View Post
If SOUND is not defined and sound.h is included (which it is via ui-prefs.h), then the stub version of register_sound_pref_parser() will be included in angband.o

If SOUND is defined, then the platform specific code needs to define register_sound_pref_parser()

The obvious solution is to make sure SOUND is not defined when unit tests are build (since, as you say, no platform specific code is compiled for the unit tests)

I'm assuming the 'unit tests' define a special 'platform specific main' (the platform being the unit test suite). So of course, we could always define some kind of dummy sound module for the unit tests (which would be rather useful for testing the parsing of the sound preferences file)
This would require building two separate 'angband.o's, one for unit tests and one for the main frontends, which the build system doesn't support - it would require a reconfigure, make clean, and build again.

There is a simple solution to this, which is to treat the sound config separately to pref files, and then you don't need all this conditional compilation and inline function stuff.
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Old April 9, 2016, 17:29   #104
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Quote:
Originally Posted by takkaria View Post
There is a simple solution to this, which is to treat the sound config separately to pref files, and then you don't need all this conditional compilation and inline function stuff.
Which is kind of how we got into the mess we're in to begin with The whole point of this rework is to guarantee that all platforms process the sound configuration exactly the same way, using the standard parser we already have.

Part of the reason I put register_sound_pref_parser() as an inline stub in sound.h was to prevent breakage of builds which haven't been converted to the new sound module. They can proceed to initialise sound and parse the old sound config file as they see fit (for the time being).

sound-core.c needs to be outside angband.o to avoid symbol clashes with init_sound() - Maybe I could just rename init_sound() to say init_sound_standard() and move the whole box and dice into angband.o and then all that needs to be worried about at the platform level is to add the platform's sound module to sound_modules[] in sound-core.c and the platform specific sound code (which is neatly outside angband.o)

This will mean that the code for parsing the sound config will be in the binary whether sound is enabled or not, but a) it's a fairly trivial amount of code; and b) it makes life easier for the automated building of the unit tests
calris is offline   Reply With Quote
Old April 9, 2016, 18:49   #105
takkaria
Veteran
 
takkaria's Avatar
 
Join Date: Apr 2007
Posts: 1,923
Donated: $40
takkaria is on a distinguished road
Quote:
Originally Posted by calris View Post
Which is kind of how we got into the mess we're in to begin with The whole point of this rework is to guarantee that all platforms process the sound configuration exactly the same way, using the standard parser we already have.
You misunderstand. I think the standardisation is great, but I just think there's no reason to link the sound parsing to the pref file system. But I know we disagree on this.

Quote:
sound-core.c needs to be outside angband.o to avoid symbol clashes with init_sound() - Maybe I could just rename init_sound() to say init_sound_standard() and move the whole box and dice into angband.o and then all that needs to be worried about at the platform level is to add the platform's sound module to sound_modules[] in sound-core.c and the platform specific sound code (which is neatly outside angband.o)

This will mean that the code for parsing the sound config will be in the binary whether sound is enabled or not, but a) it's a fairly trivial amount of code; and b) it makes life easier for the automated building of the unit tests
I think it makes the most sense to do that. However there's no reason to rename init_sound() - there's no frontends that use a function with that name, so there's no clashes.
__________________
takkaria whispers something about options. -more-
takkaria is offline   Reply With Quote
Old April 12, 2016, 14:19   #106
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
OK, I've pushed sound-core into angband.o - hopefully the auto builder likes it. Commit is in my sound-devel-clean branch

There is an #ifdef SOUND in sound-core.c I don't really like, and in reality it could be removed. Removing it will mean that the sound preferences parser will always be compiled into angband whether there is a front-end sound module or not. I guess that's the price we pay to keep an auto builder happy
calris is offline   Reply With Quote
Old April 12, 2016, 14:26   #107
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Um Nick, I just noticed you pulled in all the patches from my (Very) dirty sound-devel branch instead of the cleaner patches sound-devel-clean

EDIT: The end result _should_ be the same though
calris is offline   Reply With Quote
Old April 14, 2016, 14:43   #108
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 54
Posts: 7,645
Donated: $60
Nick is on a distinguished road
OK, autobuilder is happy now.

One minor issue: unit tests don't compile unless ./configure is run with --disable-sdl-mixer (which is what the autobuilder does), because now init_sound_sdl is missing from angband.o. I couldn;t see an obvious fix, but I also couldn't see much harm.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old April 16, 2016, 14:07   #109
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 54
Posts: 7,645
Donated: $60
Nick is on a distinguished road
Quote:
Originally Posted by Nick View Post
One minor issue: unit tests don't compile unless ./configure is run with --disable-sdl-mixer (which is what the autobuilder does), because now init_sound_sdl is missing from angband.o. I couldn;t see an obvious fix, but I also couldn't see much harm.
Well, there was a fix - add a stub of init_sound_sdl to the test suite, and now everyone is happy. I think we can regard this as successfully integrated

BTW, I also worked out what was wrong with the birth tests - you had removed the -s option which the test script was using
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old April 17, 2016, 01:35   #110
calris
Adept
 
Join Date: Mar 2016
Posts: 194
calris is on a distinguished road
Quote:
Originally Posted by Nick View Post
Well, there was a fix - add a stub of init_sound_sdl to the test suite, and now everyone is happy. I think we can regard this as successfully integrated
Is there any reason not to run the tests with --disable-sdl-mixer? If we want to test the parsing of the sound configuration etc., we could just add a dummy test sound module (which is essentially what you have done)

Quote:
BTW, I also worked out what was wrong with the birth tests - you had removed the -s option which the test script was using
I just had a look through the commit history and can't figure out where I did that

And does sound work in Widnows? As I said, I never got a chance to test it
calris is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
Reworking the entire magic system (yay)! TricksterWolf Development 8 September 20, 2015 20:22
Mk build system? CJNyfalt Development 11 March 25, 2015 07:25
Resist system Jungle_Boy Development 65 August 30, 2011 03:10
Combat System Sirridan Development 9 July 14, 2009 07:11
RFC: Middle Earth map for Un in ASCII; is it readable? Bandobras Variants 13 November 25, 2007 03:15


All times are GMT +1. The time now is 05:59.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.