search

Cabal-Syndicate / randora_framework

Framework for the Randora Game Engine
Other
3 stars 1 forks source link

RNDOwned mixin refactoring #24

Closed mattwollf closed 9 years ago

mattwollf commented 9 years ago

PR for #16

Master-Foo commented 9 years ago

I'm on vacation right now for a few more days. I'll take a look at it as soon as I can though.

Thanks!

mattwollf commented 9 years ago

That's fine, I was surprised I hadn't heard from you in a while since I've been slow too with school and work. Enjoy the rest of your vacation!

On Thu, Sep 17, 2015 at 9:03 PM, Master Foo notifications@github.com wrote:

I'm on vacation right now for a few more days. I'll take a look at it as soon as I can though.

Thanks!

— Reply to this email directly or view it on GitHub https://github.com/Cabal-Syndicate/randora_framework/pull/24#issuecomment-141298880 .

Master-Foo commented 9 years ago

Yeah, I figure that if someone wants to help it's because they want to help and if they don't, they won't. So, nagging isn't going to be a solution.

I have responsibilities other than this project as well, so I figure "do what you want, when you want".

I'm not in a position right now to really check out what you did. With this pull request, In your opinion, do you consider issue #16 complete? Or is there still more to do? Or is there still work to be done, but want to move on to some other task?

The issue isn't very sexy, so it's OK if you want to move on. I assigned it mostly so you could get an understanding of mixins. So don't feel like you have to complete it 100%.

Feel free to move on to another issue. Or, if there is something you are particularly interested in, let me know. I'll make a game plan for you.

Truth be told, I think it would be best for you and the project to find an aspect that you are interested in and train you to the point where you can be the "Resident Expert" in that aspect. This way you can feel like you have ownership in this project as well, as opposed to just being "The Intern".

It's OK if you don't think you have the skills. The important thing is that you are interested enough in it to learn.

Master-Foo commented 9 years ago

Approved. Thanks.

FYI, in the constructor mixin it is not nessesary to pass in RNDOwned into the template because everything within a mixin template shares scope with the block it was mixed in.

It doesn't hurt to have it. Maybe it's better for clarity's sake. So, I'm going to approve it. I was just wondering if you did it because your thought it was required? Or because you thought it was a good idea?

I'm going to mark it down as "A good idea". Although unnecessary.

Anyway, thanks for the help. Let me know what you want to work on next.

Master-Foo commented 9 years ago

I might add however, that instead of labeling the template as T, give it a more descriptive name, like "ObjectType".

You can change it if you want. For now I'm going to say "It's good enough".

Master-Foo commented 9 years ago

Another tip you might find useful.

if you need a generic way of getting the type of an object without specifying it outright you can use typeof(this);

You don't need to act on this information, it's just FYI.

mattwollf commented 9 years ago

much simpler reasoning for the parameterized mixin: I didn't have a full understanding of mixins when I wrote that.

Master-Foo commented 9 years ago

Do you think what you learned was valuable? If so, I won't feel so bad for assigning such a shit issue to you.

If you think it was a waste of your time. I'm sorry.

I'm going to close this issue. Let me know what you want to work on next.