infinitered / redpotion

We believe iPhone development should be clean, scalable, and fast with a language that developers not only enjoy, but actively choose. With the advent of Ruby for iPhone development the RubyMotion community has combined and tested the most active and powerful gems into a single package called RedPotion
MIT License
233 stars 40 forks source link

when using ProMotion-form on_load and similar methods are called twice #91

Closed markrickert closed 9 years ago

markrickert commented 9 years ago

I've set up a demo repo here.

https://github.com/markrickert/redpotion_bug_91

Go ahead. try it.

then comment out redpotion and bundle, run again and you'll see on_load and will_appear are only called once.

squidpunch commented 9 years ago

we've seen this kind of thing before (#37, #34).

Assuming you dont beat me to it @markrickert I'll try and check this out (and verify this hasn't returned in the areas I've already tweaked it....)

markrickert commented 9 years ago

I think the issue is in the pm_handles_delegates method.

A PM::FormScreen is not a ProMotion::ViewController or ProMotion::TableScreen

markrickert commented 9 years ago
(main)> rmq.view_controller
=> #<TestFormScreen:0x10ac886c0>
(main)> rmq.view_controller.superclass
=> ProMotion::FormScreen
(main)> rmq.view_controller.superclass.superclass
=> #<Class:#<NilClass:0x4>>

I think we can just add ProMotion::FormScreen to that list and be done with it :)

squidpunch commented 9 years ago

yeah, thats likely it - so RedPotion is firing the call, as well as the PM::FormScreen

squidpunch commented 9 years ago

yup, that's my thinking.

markrickert commented 9 years ago

I'm testing that change with a project that doesn't have PM::FormScreen... we might have an issue with it being undefined.

markrickert commented 9 years ago

yup.

2015-05-05 19:02:38.389 formotion-test[95025:4549526] ui_view_controller.rb:161:in `pm_handles_delegates?': uninitialized constant ProMotion::FormScreen (NameError)

Any idea on how to handle that? We might need to do some other kind of checking instead of checking the screen's superclass.

squidpunch commented 9 years ago

something a bit more Monkeyish...? Reopen the classes that "handle the delegates" set an attribute (or define a method) - then check that attribute or if it responds to it to decide if its handled, or if redpotion needs to handle it

something like that, perhaps?

squidpunch commented 9 years ago

So in this case, a project without PM: FormScreen we'd be defining the class, and it would be a no-op because that class isnt there or used. Its more pollution, but we've accepted some pollution in RedPotion - and this can be extracted into its own thing, not replacing existing methods - I think that might be the solution, especially for these plugin type gems

thoughts?

twerth commented 9 years ago

Hmm, yeah, good question squid. Be nice if FormScreen added something. Another option is to check for the module that's included, but I think that's a more expensive operation, but may not matter.

markrickert commented 9 years ago

I think i have a solution...running tests now.

squidpunch commented 9 years ago

yeah we could go either way, open up the class and define what we need to make this verification.

class ProMotion::ViewController
  def handles_delegates?
    true
  end
end
....

if respond_to?(:handles_delegates?) && handles_delegates?
  puts "something like this for example..."
end

Or we could check if the class we expect to exist is defined as part of the check

# this one should always be there, because of dependencies, but you get the idea.
NSClassFromString("ProMotion::ViewController").present? && self.is_a?(ProMotion::ViewController)
...continued for each type..
squidpunch commented 9 years ago

ok really heading offline later, sounds like @markrickert is running with it - I'll be back to review later if you guys want / need!

markrickert commented 9 years ago

Unfortunately, NSClassFromString() doesn't work for ruby modularized classes. :cry: