I think I found a solution for the dependency issue I mentioned. The inclusion of "ValueChangeEvent" when not otherwise specified in the project provided a useful clue here! (Otherwise I suspect I would not have found this) I need to do a bit more testing, but I expect to issue a related PR for a change in ClassDirectiveProcessor before too long.
On Mon, Sep 5, 2016 at 10:05 PM, Greg Dove <greg.d...@gmail.com> wrote: > Alex, I did some testing with a minimal project, I did not even have to > try as-only, as SimpleApplication was sufficient. And yes, there is an > issue if the dependency is not explicitly added somewhere. I will try to > find a way to resolve this, otherwise the ASCompilationUnit code will need > to go back. > I had thought it might be useful to have the bindable implementation be > more of any output variation than a manipulation of the AST. I had even > wondered about longer term possibilities with such an approach - e.g. > perhaps different binding implementation variations (e.g. as3 Signals > instead of events or something like that). I realize that this is not > something to think about seriously now. Plenty to do with things as they > are... was just thinking of possibilities for the future. > > Anyhow, if I can't find a quick solution to add a specific dependency for > output in the swf without the ASCompilationUnit AST manipulation approach, > then that will need to go back (at least for now, anyway). > Perhaps this was the reason you did this in the first place? Was it when > the EventDispatcher base class for binding became non-native? > If you think its best to try and fix the original problems with the > original code in ASCompilationUnit , then perhaps that is the simplest > route for now. If so, I'd be keen to leave the other stuff I added for a > short time, but will happily remove the unnecessary parts once we can be > sure they never get branched into as a fallback (i.e. we can be sure the > ASCompilationUnit code catches all the 'extends' candidates). Let me know > what you think. > > fyi I will be focusing on reflection work in whatever spare time I can > find this week, following on from where you got to earlier this year with > that. I will let you know where I get to later this week, I'm not sure if I > will have it finished this week or not. > > cheers > Greg > > > On Sat, Sep 3, 2016 at 1:41 PM, Greg Dove <greg.d...@gmail.com> wrote: > >> Sound great. Yes, switching it back on at any point should remain a quick >> fix option because it would simply make the other code act as a backstop. >> I will take a quick look on Monday to see if I can find any problems in >> small as-only projects where maybe the order of class definitions in the >> swf could be important with the current approach. Sounds a bit like a >> theoretical edge case but you made me think about it. >> Have a nice weekend. >> >> -Greg >> [sent from my phone] >> >> On 3/09/2016 12:41 PM, "Alex Harui" <aha...@adobe.com> wrote: >> >>> >>> >>> On 9/2/16, 5:10 PM, "Greg Dove" <greg.d...@gmail.com> wrote: >>> >>> >Yes, that was one of the major changes. I moved it alongside the other >>> >implementation inside ClassDirectiveProcessor and was able to get more >>> >consistent results. >>> >I initially commented it out so I could get everything working using the >>> >implements IEventDispatcher approach - which is the more general >>> approach >>> >that would work for all cases- in javascript, then added it back for >>> both >>> >targets as a 2nd output variation in a later commit, and it went to >>> >ClassDirectorProcessor for swf. >>> > >>> >I was seeing 2 issues from the ASCompilationUnit implementation: >>> >1. It was not always applying where it should have been (so in cases >>> where >>> >it could have been 'extends EventDispatcher' it would sometimes fall >>> >through to the implements IEventDispatcher approach in >>> >ClassDIrectiveProcessor anyway). I suspect this was threading-related, >>> but >>> >I am not so familiar with threads. >>> > >>> >2. Less of an issue, it could also apply an EventDispatcher base class >>> in >>> >static-only bindable cases, which is unnecessary, this I expect could >>> have >>> >been more easily fixed. I tried checking with hasModifier in the >>> original >>> >code, but I think that might not be available yet, iirc that caused an >>> >error. >>> > >>> >So I moved everything to the ClassDirectiveProcessor, which had the >>> other >>> >implementation of bindable support in any case. Sorry, I thought I had >>> >been >>> >clear about that. >>> >>> Well, you probably did explain it, but it probably didn't stick in my >>> head >>> until we hit this last issue and I went looking for the code where I had >>> a >>> vague memory of hacking a base class. I still don't know the compiler >>> code so well that it is clear in my head what work should be done where. >>> It looks like the original attempt to deal with [Bindable] for SWF was >>> done in ClassDirectiveProcessor, but when I wanted to fix a bug in >>> FalconJX I decided to fix it by hacking the AST since that drives >>> everything in FalconJX. The right answer may have been for me to move >>> the >>> code from ClassDirectiveProcessor into ASCompilationUnit and use the >>> existing tests for needsEventDispatcher, then maybe we wouldn't have had >>> the two issues above. >>> >>> Anyway, I think we have recorded the possibility that AST hacking might >>> be >>> a solution if we hit other problems. I'm ok with leaving the code as is >>> until we hit another problem but feel free to keep digging if you want. >>> >>> Thanks, >>> -Alex >>> >>> >
