HISE Logo Forum
    • Categories
    • Register
    • Login

    (Possible) Breaking change: local variables references inside nested function definitions

    Scheduled Pinned Locked Moved General Questions
    75 Posts 10 Posters 5.2k Views
    Loading More Posts
    • Oldest to Newest
    • Newest to Oldest
    • Most Votes
    Reply
    • Reply as topic
    Log in to reply
    This topic has been deleted. Only users with topic management privileges can see it.
    • Christoph HartC
      Christoph Hart @d.healey
      last edited by

      @d-healey This shouldn't affect this example - it would if you would define that second inline function inside the first one, but that's not possible anyway.

      1 Reply Last reply Reply Quote 1
      • d.healeyD
        d.healey
        last edited by

        I'm adding reg nest = {}; at the top of each namespace and putting my nested function variables inside there, not ideal but keeps things tidy.

        Libre Wave - Freedom respecting instruments and effects
        My Patreon - HISE tutorials
        YouTube Channel - Public HISE tutorials

        1 Reply Last reply Reply Quote 1
        • FortuneF
          Fortune @Christoph Hart
          last edited by

          @christoph-hart said in (Possible) Breaking change: local variables references inside nested function definitions:

          So the fix is pretty easy: don't allow local references in nested function calls.

          What about previously finished projects? All of them must be modified now, because you can't fix?

          Sorry but the above suggestion is not a "fix", it is a prevention workaround. I think rather than telling us to not allowing local references in nested function calls, this issue should be really fixed first. You should fix it.

          Christoph HartC 1 Reply Last reply Reply Quote 0
          • Christoph HartC
            Christoph Hart @Fortune
            last edited by

            @fortune said in (Possible) Breaking change: local variables references inside nested function definitions:

            Sorry but the above suggestion is not a "fix", it is a prevention workaround

            Of course it is a fix. There was a problem in the language that lead to unintentional behaviour so now it throws a proper error and prevents users from facing this issue.

            And if your finished project doesn't have any issues that come from this problem, you don't need to do anything, but the next time you'll update it and it start throwing these errors, you can fix them and sleep better.

            DanHD 1 Reply Last reply Reply Quote 4
            • DanHD
              DanH @Christoph Hart
              last edited by

              @christoph-hart I'm getting an error in my expansion installer callback on the line "local origin = hr;". I think this was largely copied from one of the example projects provided by @Christoph-Hart.

              Just so I'm crystal clear, do I want to be using something other than 'local'?

              I've had some users reporting that the expansion installer gets stuck trying to install the samples - I wonder if this is related...

              inline function onINSTALLEXPControl(component, value)
                  {
                      
                      if (value == 1)
                      {
                  
                local msg = "SELECT EXPANSION .hr1 FILE";
              
                Engine.showYesNoWindow("SELECT .HR1 FILE", msg, function(response)
                {
                  if (response)
                  {
                    FileSystem.browse(FileSystem.Downloads, false, "*.hr1", function(hr)
                    {
                      if (hr.toString(2) == ".hr1")
                      {
                        local origin = hr; // Scoped copy
                        msg = "SELECT A LOCATION FOR THE SAMPLE FILES";
              
                        Engine.showYesNoWindow("SAMPLE LOCATION", msg, function(response)
                        {
                          if (response)
                          {
                            FileSystem.browseForDirectory(FileSystem.Samples, function(dir)
                            {
                               expHandler.installExpansionFromPackage(origin, dir);
                            });
                          }
                        });
                      }
                    });
                  }
                });
              }
                  }
              
              Content.getComponent("INSTALLEXP").setControlCallback(onINSTALLEXPControl);
              
              d.healeyD 1 Reply Last reply Reply Quote 0
              • d.healeyD
                d.healey @DanH
                last edited by

                @danh You need to declare that variable as a reg outside of the function, put it at the top of your script/namespace

                Libre Wave - Freedom respecting instruments and effects
                My Patreon - HISE tutorials
                YouTube Channel - Public HISE tutorials

                DanHD Christoph HartC 2 Replies Last reply Reply Quote 0
                • DanHD
                  DanH @d.healey
                  last edited by

                  @d-healey ok so just:

                  reg origin = hr; // Scoped copy
                  
                  inline function onINSTALLEXPControl(component, value)
                      {
                          
                          if (value == 1)
                          {
                      
                    local msg = "SELECT EXPANSION .hr1 FILE";
                  
                    Engine.showYesNoWindow("SELECT .HR1 FILE", msg, function(response)
                    {
                      if (response)
                      {
                        FileSystem.browse(FileSystem.Downloads, false, "*.hr1", function(hr)
                        {
                          if (hr.toString(2) == ".hr1")
                          {
                            
                            msg = "SELECT A LOCATION FOR THE SAMPLE FILES";
                  
                            Engine.showYesNoWindow("SAMPLE LOCATION", msg, function(response)
                            {
                              if (response)
                              {
                                FileSystem.browseForDirectory(FileSystem.Samples, function(dir)
                                {
                                   expHandler.installExpansionFromPackage(origin, dir);
                                });
                              }
                            });
                          }
                        });
                      }
                    });
                  }
                      }
                  
                  Content.getComponent("INSTALLEXP").setControlCallback(onINSTALLEXPControl);
                  
                  d.healeyD 2 Replies Last reply Reply Quote 0
                  • Christoph HartC
                    Christoph Hart @d.healey
                    last edited by

                    @DanH yes this is example ticks all boxes for that issue, but I can't guarantee that there is nothing else wrong :)

                    But there is another code smell - you're declaring the local variable inside the nested function definition, which should not be possible (and I think the recent change will also prevent that, but I'll have to check).

                    In your case you might get away with just changing local to var for the origin definition.

                    DanHD d.healeyD 3 Replies Last reply Reply Quote 0
                    • DanHD
                      DanH @Christoph Hart
                      last edited by DanH

                      @christoph-hart yes I changed 'local origin = hr;' to 'var origin = hr;' and the errors were gone.

                      EDIT: It's all going wrong when trying to install an expansion now though :face_with_tears_of_joy:

                      1 Reply Last reply Reply Quote 0
                      • d.healeyD
                        d.healey @DanH
                        last edited by

                        @danh said in (Possible) Breaking change: local variables references inside nested function definitions:

                        if (hr.toString(2) == ".hr1")

                        Magic number alert!!! You should use hr.toString(hr.OnlyExtension)

                        Libre Wave - Freedom respecting instruments and effects
                        My Patreon - HISE tutorials
                        YouTube Channel - Public HISE tutorials

                        DanHD 1 Reply Last reply Reply Quote 0
                        • d.healeyD
                          d.healey @DanH
                          last edited by d.healey

                          @danh said in (Possible) Breaking change: local variables references inside nested function definitions:

                          ok so just:

                          Yes, but it's no longer scoped now so you can change that comment ;)

                          Also hr does exist at init so you can't assign the value there, you have to do that within your function like you were before.

                          Libre Wave - Freedom respecting instruments and effects
                          My Patreon - HISE tutorials
                          YouTube Channel - Public HISE tutorials

                          DanHD 1 Reply Last reply Reply Quote 0
                          • DanHD
                            DanH @d.healey
                            last edited by

                            @d-healey said in (Possible) Breaking change: local variables references inside nested function definitions:

                            Also hr does exist at init so you can't assign the value there, you have to do that within your function like you were before.

                            with 'var'?

                            d.healeyD 1 Reply Last reply Reply Quote 0
                            • d.healeyD
                              d.healey @DanH
                              last edited by

                              @danh I'd say use a reg but if Christoph says a var is okay then var is ok. My point was you have reg origin = hr at the top of your script now, but hr doesn't exist until your FileSystem.browse call, so you can't assign it when you declare the origin variable at the top of your script you have to assign it after the call to browse just like you were with local origin = hr now you just put origin = hr. Or do what Christoph said.

                              Libre Wave - Freedom respecting instruments and effects
                              My Patreon - HISE tutorials
                              YouTube Channel - Public HISE tutorials

                              1 Reply Last reply Reply Quote 1
                              • DanHD
                                DanH @Christoph Hart
                                last edited by

                                This post is deleted!
                                1 Reply Last reply Reply Quote 0
                                • DanHD
                                  DanH @d.healey
                                  last edited by

                                  @d-healey said in (Possible) Breaking change: local variables references inside nested function definitions:

                                  Magic number alert!!! You should use hr.toString(hr.OnlyExtension)

                                  Right, changing to the above makes the install process stop before the window appears allowing me to select the destination for the samples. So... select the .hr1 file and then nothing more...

                                  d.healeyD 1 Reply Last reply Reply Quote 0
                                  • d.healeyD
                                    d.healey @DanH
                                    last edited by

                                    @danh Oo could be that the constant is bugged. I'm away from the computer for the next day so can't test, stick to your magic number for now :)

                                    Libre Wave - Freedom respecting instruments and effects
                                    My Patreon - HISE tutorials
                                    YouTube Channel - Public HISE tutorials

                                    1 Reply Last reply Reply Quote 0
                                    • ustkU
                                      ustk
                                      last edited by

                                      hr was already external, right? so why bother using another variable origin for the same thing in the first place?
                                      It might be me, everyone around is telling me I need glasses... Arfff... 44... might be the time anyway...

                                      d.healeyD 1 Reply Last reply Reply Quote 0
                                      • d.healeyD
                                        d.healey @ustk
                                        last edited by

                                        @ustk hr comes from the file browse call but the scope doesn't extend to the second browse call so it needs to be stored in a temporary variable

                                        Libre Wave - Freedom respecting instruments and effects
                                        My Patreon - HISE tutorials
                                        YouTube Channel - Public HISE tutorials

                                        1 Reply Last reply Reply Quote 0
                                        • ustkU
                                          ustk
                                          last edited by ustk

                                          Just to say that I updated my custom branch, and all my var changed to local in the current project that is dependent on this very custom branch.
                                          It's not a big deal to change back everything though as I remember this fix. But in the case it can be fixed it could help other people not to have this surprise, without knowing where it comes from :)

                                          orangeO 1 Reply Last reply Reply Quote 1
                                          • ustkU
                                            ustk
                                            last edited by ustk

                                            This post is deleted!
                                            1 Reply Last reply Reply Quote 0
                                            • First post
                                              Last post

                                            16

                                            Online

                                            1.7k

                                            Users

                                            11.9k

                                            Topics

                                            103.4k

                                            Posts