Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ModelObserver: onSaveModel / onPostSaveModel wrong path in case of SaveAs. #8

Open
prachtan opened this issue Nov 1, 2017 · 10 comments

Comments

@prachtan
Copy link

prachtan commented Nov 1, 2017

1). RubyAPI - SketchUp 2017,
2). Windows.

Sketchup.active_model.path is not updated properly in case of SaveAs - ModelObserver is unable to get new model.path.

module Issues
    module IssueSaveAs

    class MyModelObserver < Sketchup::ModelObserver

        def onPostSaveModel(model)
            puts "onPostSaveModel: #{model.path}"
        end

        def onPreSaveModel(model)
            puts "onPreSaveModel: #{model.path}"
        end

        def onSaveModel(model)
            puts "onSaveModel: #{model.path}"
        end
    end

    def self.Start
        model = Sketchup.active_model
        model.add_observer(MyModelObserver.new)

        puts "Current file path is #{model.path}"
        puts "Now SaveAs current file."
    end

    unless file_loaded?(__FILE__)
        menu = UI.menu('Plugins')
        menu.add_item('SaveAs Issue') {
            self.Start
        }
        file_loaded(__FILE__)
    end
    end # module IssueSaveAs
end # module Issues

Simple plugin:
https://www.dropbox.com/s/36gmiw0bfo6njos/ModelObserver_SaveAs.rbz?dl=0

@thomthom thomthom changed the title [RubyAPI] [bug] ModelObserver: onSaveModel / onPostSaveModel wrong path in case of SaveAs. ModelObserver: onSaveModel / onPostSaveModel wrong path in case of SaveAs. Nov 2, 2017
@thomthom
Copy link
Member

Logged as SU-38305

@LItterBoy-GB
Copy link

@thomthom When will this bug be fixed?

@DanRathbun
Copy link

With the timer block delay suggested by @Eneroth3 ...

module Issue8

  class ModelSpy < Sketchup::ModelObserver
    # Attach this observer to the model object.
    def attach(model = Sketchup.active_model)
      model.add_observer(self)
    end
  ### AppObserver callbacks:
  #
    def expectsStartupModelNotifications
      return true
    end
    def onNewModel(model)
      attach(model)
    end
    def onOpenModel(model)
      attach(model)
    end
  ### ModelObserver callbacks:
  #
    def onSaveModel(model)
      puts "onSaveModel (no delay): #{model.path.inspect}"
      UI.start_timer(1.0,false) {
        puts "onSaveModel (delayed): #{model.path.inspect}"
      }
    end
    def onPostSaveModel(model)
      puts "onPostSaveModel (no delay): #{model.path.inspect}"
      UI.start_timer(1.0,false) {
        puts "onPostSaveModel (delayed): #{model.path.inspect}"
      }
    end
  end

  if !defined?(@loaded)
    # Instantiate and attach an new ModelSpy to SketchUp:
    Sketchup.add_observer(ModelSpy.new)
    @loaded = true
  end

end

... the following is returned ...

onSaveModel (no delay): "C:\\Users\\Dan\\Documents\\test_def_save_as.skp"
onPostSaveModel (no delay): "C:\\Users\\Dan\\Documents\\test_def_save_as.skp"
onSaveModel (delayed): "C:\\Users\\Dan\\Documents\\test_def_save_as_with_new_name.skp"
onPostSaveModel (delayed): "C:\\Users\\Dan\\Documents\\test_def_save_as_with_new_name.skp"

~

@LItterBoy-GB
Copy link

LItterBoy-GB commented Aug 5, 2021

With the timer block delay suggested by @Eneroth3 ...

Thank you. But this plan can't completely solve my problem.

I open ‘a.skp’, and I modify the model, then I open ‘b.skp’.
The save is triggered.
When I use UI.start_timer(0), the Sketchup.active_model.path is ‘b.skp’, not ‘a.skp’.
So it’s no use using a timer.
I think the method onSaveModel should be called after the Sketchup.active_model.path is updated.

@DanRathbun
Copy link

DanRathbun commented Aug 5, 2021

I think the method onSaveModel should be called after the Sketchup.active_model.path is updated.

I think we all agree that ALL the model object's properties should to be updated before the observer callbacks fire.

I open ‘a.skp’, and I modify the model, then I open ‘b.skp’. The save is triggered.

On Windows the save for "a.skp" should occur before the file browser ever let's the user select "b.skp".

When I use UI.start_timer(0), the Sketchup.active_model.path is ‘b.skp’, not ‘a.skp’.

Again, nobody ever suggested you call UI.start_timer with a delay of 0.
It makes no sense and is the same as not using a timer block as there would be no delay.

Are we misunderstanding your problem ? Can you post a reproduceable code snippet ?

@thomthom
Copy link
Member

thomthom commented Aug 5, 2021

Again, nobody ever suggested you call UI.start_timer with a delay of 0.
It makes no sense and is the same as not using a timer block as there would be no delay.

A timer with a value of 0 is not the same as not having a timer. The code in the timer block will execute after the current task on the main thread is done.

puts "Before timer line"
UI.start_timer(0, false) { puts "Hello Timer" }
puts "After timer line"

Output:

Before timer line
After timer line
Hello Timer

@DanRathbun
Copy link

Okay. I stand technically corrected.

However this fact still will not give enough delay to allow the model's properties to be updated as the engine is still working it's way through the Model "onSave" callback queue, and the timer block will run before @LItterBoy-GB's callback returns.

So we could modify what I said to be ... the use of a timer with a 0 delay only delays the block until the end of the callback.

So it is really "six and one-half dozen the other".

@LItterBoy-GB
Copy link

@DanRathbun I'm very sorry for my previous reply was too messy.

I want to explain that timer can't solve all the problems of saving. I hope the government can pay attention to this issue.

new file => save 'a,skp'

model.path=>a.skp 
Sketchup.active_model.path=>a.skp

open 'a.skp' => save as 'b.skp'

model.path=>b.skp
Sketchup.active_model.path=>b.skp

open 'a.skp' =>modify 'a,skp' => new file
The action 'save a.skp' triggered.

model.path => 
Sketchup.active_model.path => 

open 'a.skp' =>modify 'a,skp' => open 'b.skp'
The action 'save a.skp' triggered.

model.path => b.skp
Sketchup.active_model.path => b.skp
class TestOb
def onSaveModel(model)

    id = UI.start_timer(0.001, false) do
      UI.stop_timer id
      puts "model.active_path=>#{model.path}"
      puts "Sketchup.active_model.active_path=>#{Sketchup.active_model.path}"
    end
  end

  def onNewModel(model)
    model.add_observer(self)
  end

  def onOpenModel(model)
    model.add_observer(self)
  end
end

test_ob = TestOb.new
Sketchup.add_observer(test_ob)
Sketchup.active_model.add_observer(test_ob)

@DanRathbun
Copy link

a. You do not need to use UI.stop_timer if the block is only going to run once.

b. Your code snippet is setting the delay to ONE MILLISECOND !

Once again, any onSaveModel callback using a UI timer must allow the SketchUp engine enough time to finish all the onSaveModel callbacks and then update the model properties !!

c. I CLEARLY showed that setting a full 1 second delay was enough of a delay to get the proper model path (with a small number of extensions loaded.)

It MAY take even more of a delay if many extensions attach model observers with onSaveModel callbacks.
To count objects with callbacks (but not necessarily attached observer objects) ...

def count_callbacks
  num = 0
  ObjectSpace.each_object {|o|
    num += 1 if o.respond_to?(:onSaveModel)||o.respond_to?(:onPostSaveModel) 
  }
  return num
end

@Fredosixx
Copy link

Fredosixx commented Jan 27, 2024

I would add that the problem persists in SU2023.1 even with the onPostSaveModel events. The model path is still not updated.

Consider this code:

def save_observer_launch
	Sketchup.add_observer MyObserver.new
end

class MyObserver
	#Sketchup events
	def expectsStartupModelNotifications ; true ; end	
	def onNewModel(model) ; model.add_observer(self) ; end
	def onOpenModel(model) ; model.add_observer(self) ; end

	#Model events
	def onPreSaveModel(model) ; puts "PRE-SAVE: #{model.path.inspect}" ; end
	def onSaveModel(model) ; puts "ON-SAVE: #{model.path.inspect}" ; end
	def onPostSaveModel(model) ; puts "POST-SAVE: #{model.path.inspect}" ; end
end	

When saving a New model to a given path, this produces:

PRE-SAVE: ""
ON-SAVE: ""
POST-SAVE: ""

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants