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

SF::View#dup broken #34

Open
vypxl opened this issue May 14, 2020 · 8 comments
Open

SF::View#dup broken #34

vypxl opened this issue May 14, 2020 · 8 comments

Comments

@vypxl
Copy link

vypxl commented May 14, 2020

Hi, it seems like SF::View#dup is broken, because it tries to call initialize of either SF::View or SF::View::Reference (which appears twice in src/graphics/obj.cr btw), both of which do not accept a SF::View instance as a single parameter.

@vypxl
Copy link
Author

vypxl commented May 14, 2020

I stumbled upon this while trying to replace views. I tried to temporarily save window.view, then replace it, render stuff, and then setting it back to the old view. But by then, the old view got modified somehow?! so I tried to dup it.

@oprypin
Copy link
Owner

oprypin commented May 14, 2020

Hmm? Seems to work

require "crsfml"

a = SF::View.new
a.reset(SF.float_rect(100, 100, 400, 200))

b = a.dup
p! b.size # => SF::Vector2(Float32)(@x=400.0, @y=200.0)

@oprypin
Copy link
Owner

oprypin commented May 14, 2020

I think it's SF::View::Reference#dup that is broken, yeah.

require "crsfml"

w = SF::RenderWindow.new

a = w.view
p! a.size # => SF::Vector2(Float32)(@x=1000.0, @y=1000.0)

b = a.dup
p! b.size # Error: wrong number of arguments for 'SF::View::Reference.new' (given 1, expected 2)

@vypxl
Copy link
Author

vypxl commented May 14, 2020

Somehow, SF::View#dup gets broken too, but only if it is a instance member or a class:

require "crsfml"

class Test
  @view : SF::View

  def initialize
    @view = SF::View.new
  end

  def try_dup
    @view.dup
  end
end

t = Test.new
p! t.try_dup # Error: wrong number of arguments for 'SF::View#initialize' (given 1, expected 2, 0) / In lib/crsfml/src/graphics/obj.cr:4463:27

@oprypin
Copy link
Owner

oprypin commented May 14, 2020

Wow, now that could very well be a Crystal bug. Thanks, I'll check that separately, even though my fix would've gotten both.

@oprypin
Copy link
Owner

oprypin commented May 14, 2020

So I reduced the sub-issue to this:

class Foo
  def initialize()
  end
  def initialize(copy : Foo)
  end
  def dup() : self
    return typeof(self).new(self)
  end
end

class Bar < Foo
  def initialize(x : Int32)
  end
end

class Test
  @foo : Foo = Foo.new

  def foo_dup
    @foo.dup
  end
end

Test.new.foo_dup
 7 | return typeof(self).new(self)
                         ^--
Error: wrong number of arguments for 'Foo#initialize' (given 1, expected 0)

And I concluded that this is not a bug. It happens due to Crystal's reduction of a type to the whole class hierarchy. I suppose these conditions cause typeof(self) to end up being "Foo or any subclass" rather than just "Foo". And then indeed you can't initialize "Foo or any subclass" with 1 arg because not all of them support initialization with 1 arg. So first off, I should've used self.class instead of typeof(self), but of course it's irrelevant anymore, because now I just use the explicit class name instead (see the fixing commit).

@vypxl
Copy link
Author

vypxl commented May 14, 2020

Certainly interesting

@Hadeweka
Copy link

I think the fix actually introduced a new bug related to duplicating certain objects derived from abstract structs.

The following code leads to an error:

a = SF::Event::Resized.new
b = a.dup

Error: can't instantiate abstract struct SF::Event::SizeEvent

Admittedly, there are few scenarios in which you'd want to copy an event, but I stumbled upon this while trying to bind crSFML to Ruby using Anyolite. I can construct a workaround, but it still might be an issue unless this is intended in some way.

@oprypin oprypin reopened this Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants