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

The bounds field of a Morph is a Rectangle but it is being used as a method #24

Open
ghost opened this issue Feb 18, 2015 · 13 comments
Open

Comments

@ghost
Copy link

ghost commented Feb 18, 2015

I noticed Morph.prototype.drawOn, that when aRect evaluates to false, it uses the bounds field as a method.
This must be fixed at line 2623 by removing the method call.

@davidedc
Copy link

makes sense. Potentially a copy/paste error from https://github.com/jmoenig/morphic.js/blob/master/morphic.js#L2694 ?

Did you find this by code inspection, right?

I mean you didn't find this because of a runtime error doing something, right? (In which case I'd be curious how this was caused).

@davidedc
Copy link

I'm asking because it would look like that false-check of aRect is never triggered, otherwise someone would have caught this earlier.

So if you didn't find this via runtime error then I'm guessing that this whole check could be removed.

davidedc added a commit to davidedc/Fizzygum that referenced this issue Feb 19, 2015
@davidedc
Copy link

runs fine in my tests without that check, I took it away from Zombie Kernel. Keeping rectangle = aRect || this.bounds; would allow some shorthands but they are evidently not used now...

@ghost
Copy link
Author

ghost commented Feb 21, 2015

They are usable if you directly use Morph.prototype.drawOn as follows:

Morph.prototype.drawOn.call(this, this.world().worldCanvas);

The check is crucial in this situation (I use it often).

@davidedc
Copy link

Ah got it. that's a big repaint though. Why do you need to repaint the whole screen, out of curiosity?
update I mean it's not necessarily a big repaint, it just appears to be out-of-pattern to paint without specifying a broken rectangle - how are you using this pattern?

@ghost
Copy link
Author

ghost commented Feb 22, 2015

Because the method specifies the rectangle as being the bounds. The morph may be any size.

@davidedc
Copy link

I understand that. I don't understand where and why you are using it. The way you are using the morph would appear in front of everything, in the canvas. The overlaps wouldn't be managed correctly. Could you send me the link here on github to the piece of code where you invoke this without a rectangle, so I can figure out the context of what you are doing?

@davidedc
Copy link

right OK I took a look into your code Luis to get to the bottom of this - I see what you are doing here https://github.com/luismark/luismark.github.io/blob/master/starlings/starlings.html#L280 .

You repaint the whole screen (and hence all the morphs in it) every ms here https://github.com/luismark/luismark.github.io/blob/master/starlings/starlings.html#L473 .

It works for you, that's probably the thing to do in a sprite-based game where it wouldn't be worth to have dozens of small damage areas, so all is good, in general though in a desktop environment this would be a very slow repaint to do every frame.

There is an easy way to achieve the same by just "damaging" the whole world, i.e. doing something like [whateverPointsToYourWorldMorph].changed() . This would create a huge damage rectangle on the whole screen and do the same of what you are doing now but without code changes.

In that case it'd also be good to check that none of the morphs create further "damage rectangles" as those would be redundant and just slow things down - although Jens has added a routine that might be merging redundant rectangles together, I'm not sure.

@jmoenig
Copy link
Owner

jmoenig commented Feb 23, 2015

Yep, that's right, damaged rectangles that are "near" to each other get merged. I love your solution, @davidedc !

@ghost
Copy link
Author

ghost commented Feb 23, 2015

I'd like to use commit codes, like luismark/luismark.github.io@790546ca35ca instead of luismark/luismark.github.io:master to indicate code lines.

@ghost
Copy link
Author

ghost commented Feb 23, 2015

I've got problems with my game! Please help me out, Jens!

@jmoenig
Copy link
Owner

jmoenig commented Feb 23, 2015

Hmm... sorry you're stuck. Can you explain what you're trying to accomplish that doesn't work the way you'd expect it to?

@ghost
Copy link
Author

ghost commented Feb 24, 2015

See issues luismark/luismark.github.io#3 and luismark/luismark.github.io#4 to understand my problem.
I need my game complete in the 28th of May of 2015.

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

2 participants