Author Message
elkano

Joined: 19 Jun 2009
Posts: 24

Posted: Sat Oct 02, 2010 5:31 pm    Post subject: Fast look into arc2-alpha code

I was answering to the following post in the announcement, but I think it's already hijacked enough, so I start a new topic for this.

 SirAlaran wrote: I've uploaded what I have so far to SVN. http://www.dsource.org/projects/arclib/browser/branches/arc2-alpha/arc If you see something in the API that looks stupid, let me know. The earlier we fix things, the easier it will be.

I am just going to list things I see on my first reading, small things, but as you say, better fix them early:

math/angle.d:
* HALF_PI is defined in std.math, which you import, as PI_2, so it's redundant.
* You define TWO_PI and then don't use it on line 48.

math/rect.d:
* In method intersects(r), you may want the defined getters instead of recalculate the coordinates and multiply the property calls.

math/vect.d
* rotateCopy(pivot, angle) is wrong, I'd say. You shoud first translate the vector by -pivot, then rotate, then retranslate by pivot:
 Code: Vector2d v = Vector2d(m_x - pivot.x(), m_y - pivot.y()); v = v.rotate(angle); return Vector2d(v.x() + pivot.x(), v.y() + pivot.y());

I use v.x instead of v.m_x because you used the properties mixin which should have created the getters. This bit would apply to most of the code, but I won't quote every part. A quick find&replace can take care of it.

* Your vector maths confused me: As far as I know, the cross product of two vectors is a vector, not a number. I also know of scalar multiplication, but not scalar cross product. And you cross(s) and bCross(s) methods are exactly the same code. There may be things I don't understand here though, my maths degree is getting old.
* I am not sure if there is a reason why distance is not in arcfl, or better, T instead of float, but it looks weird.
* in projectOnto, I imagine x,y are m_x, m_y. Also, f = dp/v.length is shorter and faster than manual calculation. Similarly, you can write 'return f*v', I think.
* forAngle and fromPolar are redundant, the only difference being that forAngle has a default length of 1.
* rotateAbout looks the same as rotateCopy(pivot, angle) to me.

A most consistant comment syntax is advisable for automatic docs. Ddoc syntax is probably the most appropriate.
Haven't looked into graphics/ yet, it's getting late. I'll do it another day.
SirAlaran

Joined: 19 Feb 2007
Posts: 84
Location: Silicon Valley

Posted: Sat Oct 02, 2010 6:26 pm    Post subject:

Ha. The math/vector code was the stuff that I ported over from Arc 1. The graphics code is where most of my work has gone. I'll fix those issues though.

On the subject of the vector code, I don't know what some of those are trying to accomplish either. There was some talk on the D newsgroups about putting a vector type in the standard library. I wonder what will become of that? Another thing I've considered is just stealing the vector code from Blaze.

For the rectangle code, were you suggesting something like this?

 Code: /**  * Returns: true if r intersects this rectangle, false otherwise  */ bool intersects(ref const Rect r) const {     if(r.right <= left)         return false;     if(r.left >= r.right)         return false;     if(r.bottom <= top)         return false;     if(r.top >= bottom)         return false;     return true }

_________________
Current projects: Project Fermitas, Arctographer tile map editor, Arclib game library.
elkano

Joined: 19 Jun 2009
Posts: 24

 Posted: Sun Oct 03, 2010 5:17 am    Post subject: Exactly that, but for the line "if(r.left >= r.right)" where there is a 'r.' too many.
SirAlaran

Joined: 19 Feb 2007
Posts: 84
Location: Silicon Valley

Posted: Wed Oct 13, 2010 11:27 pm    Post subject:

Continuing from here
 Krendil wrote: Hi, I just started using arc2-alpha, and I've found a couple little problems (You may have fixed already, but here they are anyway) In graphics/draw.d lines 49-59, x and y are cast to int about half the time, and left alone the other half. This causes problems when you try and call the function with x and y as floats or doubles. I don't know why they are being cast in the first place, but they should either all become ints or all left alone.

I changed them to cast(T). The reasoning there is that the texture width and height are uint.
 Quote: In math/vector2d.d, opAssign is overloaded for int[2u], but this prevents vectors being assigned to other vectors. I think this might be difference between D1 and D2. I tried adding another opAssign function for vectors, and that seemed to work.

Fixed in SVN.
 Quote: In math/rect.d lines 55 to 83, the compiler gives an error along the lines of "function is declared nothrow yet may throw". Apparently a lot more things throw exceptions in D2, including memory allocations. I can't see anything in there that might throw an exception, unless it doesn't like the @property attributes or mixins.

I'm still investigating that. I don't see how they could throw either, and yet the compiler complains. I even added "nothrow" to the property generator.
 Quote: This isn't a bug in arclib, but I just thought I'd point out that dsss/rebuild has a bug where files with @property in them (like vector2d) aren't imported or processed properly, causing linker errors. You can get around this by importing them from one of the files you give to rebuild. Bug report here. Also, are there any plans to add a function that only draws part of a texture? Or creating a texture that is a reference to a subsection of another texture? Either of these would be useful for implementing spritesheets.

Yes, and I'll be using it extensively in the tilemap module (when I get around to writing it)
 Quote: Anyway, this looks like a great, useful project. Keep up the good work.

_________________
Current projects: Project Fermitas, Arctographer tile map editor, Arclib game library.