RFC: Reworking sound sub-system

Collapse
X
 
  • Time
  • Show
Clear All
new posts
  • calris
    Adept
    • Mar 2016
    • 194

    #76
    Originally posted by Nick
    You have to have configured with --enable-test, then
    Code:
    ./run-tests
    for birth tests,
    OK - so how do I figure out why the tests failed. And did they fail before my work on the sound modules (not sure how my work could impact in birth)

    Comment

    • calris
      Adept
      • Mar 2016
      • 194

      #77
      And what are the opinions regarding #include in .h files?

      The Windows build error is due to win-layout.c needing ui-keymap.h which needs ang_file which is defined in z-file.h

      ui-keymap.h does not have any #includes, but I notice others, like ui-prefs.h, do.

      I like to include any necessary headers in headers that need them, but I've worked with people that think this is a bad idea (I think mainly for dependency tracking).

      My argument (and the argument of a lot of developers) is that if you add functionality to some code that needs definitions in foo.h then you should only ever need to #include fooh. - foo.h will #include everything foo.h needs in order to compile.

      Comment

      • calris
        Adept
        • Mar 2016
        • 194

        #78
        Originally posted by Nick
        Windows only has one missing #include, by the looks:
        Code:
        In file included from ./ui-prefs.h:25:0,
                         from win/win-layout.c:21:
        ./ui-keymap.h:60:18: error: unknown type name �ang_file’
         void keymap_dump(ang_file *fff);
                          ^
        Makefile.win:134: recipe for target 'win/win-layout.o' failed
        Added #include "z-file.h" to ui-keymap.h to resolve this

        Also fixed an oversight with the new handling of supported file type hook

        Pushed to sound-devel

        I think I'm getting closer to the finish line - When we are all happy, I will trash sound-devel and develop a better set of patches that apply the changes incrementally, somthing like:
        - Add sound-core
        - Migrate Linux sound
        - Migrate Windows sound

        Comment

        • Nick
          Vanilla maintainer
          • Apr 2007
          • 9634

          #79
          Originally posted by calris
          OK - so how do I figure out why the tests failed. And did they fail before my work on the sound modules (not sure how my work could impact in birth)
          I've been wondering the same thing recently - unit tests are in src/tests/bin/whatever and can be run with gdb, but I'm not sure how to debug an individual birth test. Maybe someone else knows.
          One for the Dark Lord on his dark throne
          In the Land of Mordor where the Shadows lie.

          Comment

          • calris
            Adept
            • Mar 2016
            • 194

            #80
            Originally posted by Nick
            I've been wondering the same thing recently - unit tests are in src/tests/bin/whatever and can be run with gdb, but I'm not sure how to debug an individual birth test. Maybe someone else knows.
            OK, looks like the birth test creates a new character, and tests that, at the end of the test, it is Level 1 with the same race and class specified by the test. Really not sure how it could fail TBH

            Comment

            • calris
              Adept
              • Mar 2016
              • 194

              #81
              Nick, I've created a new branch for the sound sub system here

              Let me know if it's to your liking and I'll generate a pull request

              EDIT: Damb - just noticed there is no commit messages - I'll fix that and push them again (EDIT: Done)

              Comment

              • Nick
                Vanilla maintainer
                • Apr 2007
                • 9634

                #82
                Originally posted by calris
                Nick, I've created a new branch for the sound sub system here

                Let me know if it's to your liking and I'll generate a pull request

                EDIT: Damb - just noticed there is no commit messages - I'll fix that and push them again (EDIT: Done)
                That's still failing on not declaring parse_prefs_dummy for me; if I add
                Code:
                #include "ui-prefs.h"
                to sounds.h, then it fails on declaring register_sound_pref_parser twice.

                It's probably related to what switches are set for ./configure; it needs to work with all combinations.
                One for the Dark Lord on his dark throne
                In the Land of Mordor where the Shadows lie.

                Comment

                • calris
                  Adept
                  • Mar 2016
                  • 194

                  #83
                  Originally posted by Nick
                  That's still failing on not declaring parse_prefs_dummy for me; if I add
                  Code:
                  #include "ui-prefs.h"
                  to sounds.h, then it fails on declaring register_sound_pref_parser twice.

                  It's probably related to what switches are set for ./configure; it needs to work with all combinations.
                  Is that Windows build? I don't have any build tools for anything but Linux, so it's a bit hard for me to test non-Linux builds

                  Comment

                  • Nick
                    Vanilla maintainer
                    • Apr 2007
                    • 9634

                    #84
                    Originally posted by calris
                    Is that Windows build? I don't have any build tools for anything but Linux, so it's a bit hard for me to test non-Linux builds
                    No, that's linux build, with (IIRC) --enable-sdl --enable-sdl-mixer --enable-stats --enable-test.
                    One for the Dark Lord on his dark throne
                    In the Land of Mordor where the Shadows lie.

                    Comment

                    • calris
                      Adept
                      • Mar 2016
                      • 194

                      #85
                      Originally posted by Nick
                      No, that's linux build, with (IIRC) --enable-sdl --enable-sdl-mixer --enable-stats --enable-test.
                      Oops - I'll check it out.

                      Apart from build failures, are you happy with the structure of the new sound module and the coding style?

                      Comment

                      • Nick
                        Vanilla maintainer
                        • Apr 2007
                        • 9634

                        #86
                        Originally posted by calris
                        Apart from build failures, are you happy with the structure of the new sound module and the coding style?
                        Yeah, absolutely
                        One for the Dark Lord on his dark throne
                        In the Land of Mordor where the Shadows lie.

                        Comment

                        • calris
                          Adept
                          • Mar 2016
                          • 194

                          #87
                          Hmmm, I just did the following for each patch in order:

                          # make distclean
                          # ./autogen.sh
                          # ./configure --enable-sdl --enable-sdl-mixer --enable-stats --enable-test
                          # make

                          And every one compiles clean

                          Comment

                          • calris
                            Adept
                            • Mar 2016
                            • 194

                            #88
                            Ah fudgicles...

                            Looks like my latest system update (to bring in sqlite to enable compiling with stats) has borked SDL

                            Can't get sound to work on any of my (previously working) dev branches. Quiting hangs Angband and Ctrl-C causes a segfault somewhere in the SDL library.

                            EDIT: SDL Graphics still works

                            EDIT 2: A reboot solved it
                            Last edited by calris; April 3, 2016, 13:12.

                            Comment

                            • calris
                              Adept
                              • Mar 2016
                              • 194

                              #89
                              Originally posted by Nick
                              That's still failing on not declaring parse_prefs_dummy for me; if I add
                              Code:
                              #include "ui-prefs.h"
                              to sounds.h, then it fails on declaring register_sound_pref_parser twice.

                              It's probably related to what switches are set for ./configure; it needs to work with all combinations.
                              register_sound_pref_parser() is only defined in sound.h if SOUND is NOT defined
                              register_sound_pref_parser() is defined in sound-core.c, but sound-core.c should only compile if SOUND IS defined

                              So somehow you managed to get SOUND to be both DEFINED and UNDEFINED.

                              Are you compiling on a Quantum computer?

                              Comment

                              • calris
                                Adept
                                • Mar 2016
                                • 194

                                #90
                                The plot thickens...

                                I just switched to the Rune ID branch and cherry picked my sound code into it (applies cleanly). I did a 'make clean', not a 'make distclean' before ./configure and got this when I went to do the make:

                                Code:
                                In file included from snd-sdl.c:23:0:
                                sound.h: In function ‘register_sound_pref_parser’:
                                sound.h:71:41: error: ‘parse_prefs_dummy’ undeclared (first use in this function)
                                  return parser_reg(p, SOUND_PRF_FORMAT, parse_prefs_dummy);
                                                                         ^
                                sound.h:71:41: note: each undeclared identifier is reported only once for each function it appears in
                                sound.h:72:1: warning: control reaches end of non-void function [-Wreturn-type]
                                 }
                                 ^
                                Failed to compile snd-sdl.c!
                                If I do a 'make distclean' it compiles clean and runs with sound as expected

                                Comment

                                Working...
                                😀
                                😂
                                🥰
                                😘
                                🤢
                                😎
                                😞
                                😡
                                👍
                                👎