-
Notifications
You must be signed in to change notification settings - Fork 1
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
General Conversation #3
Comments
After setting up my Fork and running your code, I found that it actually wasn't stable, so I've made a few small modifications that make it stable. For now, I have uploaded these files to enable some testing. You can view (my fork) of the project here: http://youribok.nl/micengine/gamepage.html I've also started a new file state.js, for the State Class, which will handle everything concerning saveStates and the saving/loading/etc thereof. I've got two questions:
|
Another question: I see you're using a script to fake Classes. What's the benefit of using such script over using what JS offers by default? |
The only thing I can think of is that I really prefer operators to have spaces around them for clarity. What things are you not liking? Change a file around as an example if it helps. I have included a small amount of SCB stuff for testing. The engine itself is not likely to have much content at all for use in testing, it has also been used somewhat to try to figure out what should and should not be part of the engine. I agree completely about making the engine and not worrying about SCB being put in, but there has to be a minimal amount of content to test. If you think something is beyond the necessary for testing, feel free to bring it up or make an issue, it'll just need to be a bit more specific. As for the classes: I wanted something like 'classic' inheritance. It saves a ton of redundant code. Being able to do a super() type call does so, too. I find the use of .prototype stuff very tedious and ugly and requiring a number of extra calls to do what the fake classes do. I am by no means a great javascript programmer, though, so feel free to school me if you think I'm just missing or overlooking something. There really isn't all that much code in here right now, so if you want to take a stab at something like content.js to show why the classes aren't needed, I have no problem with taking a look. I am willing to learn and admit my ignorance. |
Spaces around operators is something I personally also prefer, although I have found in the past that people think differently about what is and what isn't an operator ;) As for the SCB stuff, the main things that I disliked are the save/load function calls, and the very basic Profile system you setup. I understand you want some testing, but Instead of first somewhat including these functionality in engine.js, only to have to be fully replaced by a proper version and its own file, I generally tend to prefer to do things right the first time (thus start these out in a separate file and actually consider use cases and other scenarios). Personally I've never really had the need to recreate the class-based functionality that the function-based JS language misses. It just feels off to me. For logger.js and state.js, I've created my own objects under mice. They're much more simple to create, but these are only basic classes. All I'm really using is some form of private vs public properties (using var vs this.). For example, state is being kept in a private variable, accessible for the save/load functionality, but not for outside callers. Those can only see the actual save/load functions. Then I've also setup a basic Profile system (committing right after this post). This is a bit more elaborate perhaps in that I'm using a constructor function similar to how you create Profiles. At first glance I can't quite find any functionality from your Class system that this design misses. But then again I'm no expert myself, since you're more familiar with your Class system perhaps you can see if you're missing any functionality? And remember, besides looking at the code in my Fork, you can also test it at http://youribok.nl/micengine/gamepage.html. Try using the console to fiddle around with the mice.Profiles object for example. |
Cool. There's a whole bunch there, so it'll be later before I have the time to get into it more than my quick glance allows. |
Huge post, sorry. So it seems like the main thing on engine.js is you moved classes to their own files. Was that the main issue you had? Everything else in it looks like it's just fixes or changes to use a proper logger instead of alerts. I would love to have what are basically classes in their own files as done in most languages. The only thing is that it apparently has some effect on bandwidth and performance issues. I guess if it ends up being a big deal, the releases could repack the files. In content you say track[] is to be replaced by State.track[], but the purpose is different. The intent of track[] in content, is that it will be a property of each thing that lists the properties of it that are saved/loaded. For example in SCB most boosts would track bought, power, unlocked, etc. State.track[] looks to be more a list of the things themselves that are to be saved, and then you just turn the whole object into a string. The only problem with this is that it will almost definitely result in larger saves and much longer save strings since stringify only ignores functions, having no clear way exclude or include certain properties. I'd like to limit the saves to include only what must be kept track of. So both are probably needed, and I think the calls for stringify and parse will have to end up being more complex functions that only save properties an object has listed in it's own track[] property. Did that make sense at all? I'd like to replace mice.State itself with mice.ProfileManager, though. Well, really the content at this point is fine, so I guess I'd just like to rename it. The reason is that it isn't really a state manager as such, meaning it doesn't keep track of anything much itself or do anything to actually dictate the game state, it just kind of handles other objects and reads/writes the game state to them. The game state will basically be a property of a profile when saved. For the most part objects should keep track of themselves during actual play. This also goes along with the idea of multiple profiles. For classes, I don't want to use them for everything, which is maybe what you thought I wanted? Their use will be mostly if not entirely restricted to things that might or do get subclassed and reused as in the content object types. Normal javascript is prefered for everything else: engine, content, state, logger, etc. In other words, Class is there as a way to keep inheritence and super calls simple, not because I want everything to be written as if javascript were a class based language. Does that clear up why I wanted it? If not, please take a run at content.js and maybe boost.js to remove Class usage so I can see how you would think to do it. I made profile a Class just because I thought it might eventually be something people subclass if the engine itself doesn't. Maybe someone's game modes would require different types of profiles or something. It may turn out that the way profiles are implemented there would never be a reason to subclass it in which case getting rid of the extra class stuff would be fine. Your mice.Profiles is where I think maybe I start to differ in opinion with you. First, this seems like it is meant to put all profiles in one save. I could just be drawing conclusions I shouldn't or misinterpreting, though, because it has turned out to be a very long day for me and I"m kinda burned. Anyways, I'd like profiles separate, the only thing even seeing that there could be more than one would be the profile manager. I don't know maybe that wouldn't be great, either. Besides that, though, it is the implementation that I don't really care for. Everything is broken up and made more awkward than it needs to be to me. Profile.call(stuff) is just really ugly to me, as is the profile interface actually being essentially a private class of something that keeps track of it. I try to avoid .call and .apply if at all possible. It could be my lack of familiarity with it, but those always seem like hacks to me. If you want to call a function, you should just call a function and pass it paramaters. It's like some weird functional detachment were a method doesn't really belong to a object. The profile object is a constructor, the constructor is not a method of an object. Yes that is kind of what java is, being function based, but in this case it's like abstraction simply for the sack of abstraction and not functionality. I don't know if that made any sense. with Class commentary:
Profiles also doesn't need to be a class, just a container, Profile Manager would manage the container: I don't know. I'll do some actual code modifying your State to PM and see what the result looks with like compared to this. Also, you are starting with capitals for all your functions, I'd like to reserve starting caps for what are bascially classes: mice.State, mice.Logger, mice.ProfileManager, etc. No .method calls should look like Obj.SubObj.Method(), rather Obj.SubObj.method(). That is how normal javaScript stuff looks. Math.floor() not Math.Floor() even though the function is technically an object in javascript. Eh, everything being an object makes it extra difficult to communicate that kind of thing. :( Now, I don't mean to shoot you down outright or anything. I'll mess with stuff, take a look at this again later, and do some code in that fashion to give it another chance and see how other things look and feel. But I gotta be honest and say I am not a fan. Maybe I'm just being grumpy after the day I had, too. |
I actually forgot to reformat the engine.js file, I guess I should do that now lol. I agree with Classes having their own file (or, just anyaspect of the engine that's too big or important to include in engine.js). For the track[], most of my code so far is an early mockup and this shows it. My initial idea was to keep a list of references to variables that need to be included in a save, and just for testing I put the mice object inside. While even with this system it's possible to have only parts of an object be included (bought, power, unlocked, etc), there's no easy way to do this yet since I hadn't quite considered that. I'll see if I can write some additional code in there to easily allow partial objects to be saved, then it should provide the same functionality as your original track[]. When I saw your track[] I just immediately thought of the track idea I had myself, and at first glance they seemed similar. ;) As per the above paragraph, I don't think we'll end up needing both, just one should be able to provide all the functionality that we need. I also mainly moved it to the State object since that's where it belongs. Speaking of the State object, a simple rename is fine, I just felt that State was a nice short word that sort of conveyes what the object actually does. you see, in its core the object is supposed to handle save/load, which is basically handling persistent data which you could call state. ProfileManager is fine, just remember that you'll have to write those 14 letters every time you want to do something save/load or profile related. I think I'm not entirely able to envision the benefits that your Class will provide, so I'll just see how you develop that specific area to get a better view of its usefulness. As for the Game requiring different structure for engine-based objects - in the case of Profiles that'd basically come down to altering ProfileBase, for which we can write a function. It's quite a simple structure, just some keys and default values, so even for that I don't think a Class is necessary. But as I said above I'll wait and see. I'll also probably write the function to edit ProfileBase sometime this weekend. While I do understand the idea of multiple profiles (I brought it up myself on #2), the current save/load is incredibly basic and I simply haven't had the time to get multiple saves in, each tied to a profile. I do understand that we're looking to make a cookie and/or a localStorage object per save. The call and apply functions are something that is mostly used in the js engine itself, but can sometimes be very handy in js development. In this particular case, I could've also called the function normally, in which case it would receive two arguments instead of one. It really just makes one argument automatically into the this object which is ok there since the original this object isn't needed (or should perhaps even be avoided). I'll make a second version without .call(). The capital thing is something of personal preference, and it's exactly why I originally asked for any specific formatting that you're really keen on. I think I usually default to using capitals on public properties (mice.State.Active), and lowercase on private properties (profiles inside mice.State, can't be accessed from outside!). Honestly I'm fine with any definition for when to use caps and when not, aslong as there is one (I do hate seeing caps in one place and not in the other, seemingly at random without any use). Other than that, well, we've got a bit of a different view on some formatting/structuring, and that's exactly why I started this issue. Stuff like this must be cleared up at the start to avoid confusion later. Edit: While reformatting engine.js, these are the main differences I noticed:
All in all this list of changes really contains everything that differs, most of which is really minor but I've included since we're comparing. The list honestly ended up shorter than I initially expected. Also again, nothing HAS to change, but I just want some solid guidelines to be able to follow, aswell as to be able to expect others to stick to. |
I went ahead and did a quick going over of how state.js might look if made more like I'm used to. I doubt it runs yet again because I made a couple changes to engine meant for further coding, but I keep getting interrupted. So, yeah, that is there if you want to look at it. I haven't gotten a chance to work more with your way of coding it, but I will. This was just quick to do for comparison sake since I'm more used to it. And I haven't had a chance to read your above post, I'll get to that later, too. Just havin' a heck of day. Hopefully these couple posts haven't come off horrible or anything, I'm glad to have help on this. |
That's fine, take your time. There's no deadline or anything, right? So don't rush it! RL stuff should always be more important. I think you've also forgot to commit during that hectic day yesterday, because the project's last change was 2 days ago. I'll have a look whenever it gets added, don't worry. And I'm glad to be able to help you on this. For me this is exactly the kind of project that can keep me busy and perhaps teach me a thing or two aswell, and that's exactly what I'm looking for. That, plus it's a nice distraction from work :) |
It should be there. Check out the network view, it's labeled I'll get back into this later today. Hopefully more productively. :) Mike Galusha On Sat, Feb 8, 2014 at 6:47 AM, GeneralYouri [email protected]:
|
Ah yes I see it now, this is the first time I've ever used the network view lol. As for my stuff not showing up as authored by me.. I have no idea. The first time I opened the github application (which I now use to push commits), it did give me some error about not being able to find commits or something, but it still worked. No idea if that's relevant here. Do you have any idea how I could fix it? I am seeing 'GeneralYouri authored 17 hours ago' on my fork for example. |
I'm not entirely sure. Are you just using Git itself? You doing command line or gui? What are editing code in? It could be that you are doing things on your master instead of on a branch, except that I've seen people with an edited master still show up in their own swimlane on other github projects. I was still able to add your fork as a remote and fetch from it, and you seem to be able to commit, so it is at least working in those respects. |
And for the big post from above: I'll just leave you to the track[] stuff for now then. Like I said, I don't have much experience with JSON in javascript. The only time I've ever really used it was through java/c++ libraries and that was all kinds of different with a lot of built in functionality like "@ignore" tags for properties. We can give ProfileManager a bit shorter name, or just add an alias like mice.PM if you want. I just don't like State because layouts will basically have states, too, and those I would like to handle seperately as well as be able to easily differentiate between the two handlers. "the current save/load is incredibly basic and I simply haven't had the time to get multiple saves in" Yeah, I pretty much figured that. There is going to be a lot of stuff that is just done one tiny step at a time. I've never done something like a game engine from scratch, and not with someone else contributing this early in the programs life. I usually contribute to things already well into development or make programs with a pretty specific scope and purpose where huge chunks can be done all at once. (Design docs for the win!) Should be interesting and informative. For .call and .apply, they are part of the langauge, so I'm sure they have reason to exist. Use them if you think it is called for. Heck, .apply is actually used in the Class setup. It's likely just such an odd and foreign thing to me that I don't care for it myself. I think what we might try is doing a bit of this code with and without the Class use. It will be duplicating work, but, like you said, I think it is more important to figure this out early. In that regard, I am going to take a shot at removing the Class stuff from content.js, but I would like if you did so as well so we can see the difference (especcially since I will probably screw it up). And finally, I will add some kind of code guidelines file to the project with some of the preferences in it. I'll try to stick in some examples and stuff. Then you can take a pass at it after if there is more you would like to include. |
So when I look at either of our versions, I can still see my name in some of the commits. So for me it seems to work fine. I haven't fully thought through the whole profile/track/save/load/etc area, I think track[] might have to be in the profile at some point. We'll see. Also JSON isn't really special, it's just a very straight up way to stringify an object. Infact if you were to stringify any object into JSON, then run that as JS code, it'll be fully valid and recreate those objects. I like the idea of working with Class and without at the same time, we'll have a better view of the differences. |
Yeah, your name still shows up, but it doesn't mark the branch as belonging to you (a swim lane). That usually only happens after a merge. Again, SCB is a good example to look at. There are swim lanes for unmerged stuff, but once they are merged, the branch is grouped in with master. It doesn't seem to be breaking anything, it is just weird. |
Alright, I just really don't think I am going to like not using Classes Consider:
==VS==
And I think the top way would only get even more messy as more needs done. If I am missing something, please show me. Edit: And actually I'm not even sure how to subclass that to one, been messing a bit and can't seem to get it. It would involve some other function that copies over properties I think, or some kind of chaining maybe? I am curious to know, if you would show me. For the bottom, it's just:
|
I think we can probably manage that stuff. Class itself doesn't actually have an init, so we can probably add one that does the default assignment and checks for complex properties to overwrite only those parts given. I'll work on it later today and see what I can come up with. The Class works now as a trickle down kind of thing, looking for a property all the way 'down' to Object.prototype so information isn't duplicated a lot. It's actually how javascript does things. (You don't see all the Object.prototype functions on everything, but you have access to them. .length doesn't show when you look at an array for example, but you can use it.) The only issue with actually assigning everything may be super() calls, I'm not really clear how it is achieved in the Class code. If a loop through properties doesn't work, we'd just have to assign everything in the init function then call init.super() on subclasses. At any rate, yeah, I'll mess with it in a while. |
I realise the 'trickle down' idea, that's basically what I edited the post for. It's just that I don't normally see it used this way, and thus it took me a while to realise what was actually happening. Although, despite the fact that this might prevent duplication of some properties, in this case I don't agree with using it. You want to have a clear separation of 'actual values' and 'default values', and this should be represented in the actual object and the prototype object, respectively. Just because a value doesn't differ between these two objects doesn't mean it should be omitted. Infact, while it reduced duplication it does make it slightly slower to look the values up. So you're essentially trading setup time for execution time, however slight both values might be. That's never a thing you want to do. Also any default properties of objects normally DO show, including Array.length. They're also shown by the Chrome console, but they're shown in a different color to note that it's a special property added by the parser (just like proto in the image I included above). As for the init problem: The most likely solution will indeed be something along the lines of having a default init function that can deal with multiple levels in arrays/objects. The init function is already called by default in the Class object, which is good. Then when you're creating the Class you can optionally provide an alternative init function which, just like the normal properties, will 'overwrite' the default. In this case though the default init would have to remain accessible (ie for inits that want to do complex property assignments but also other stuff). On a different note; we're curently both working on our own versions, I feel like we gotta sync them soon or we'll have lots of conflicts. Edit : |
Yeah, that was just a result of trying out Class vs not Class stuff. It is a bit of mess, so I'll probably do a manual merge and then you'll have to grab a new master from that. After that things should be fine since there won't be alternate experiments going on. And it is interesting that those properties show up in chrome, they don't in firefox with firebug. Something like var thing= [1,2,3] shows only {0:1 1:2 2:3}. Going to lunch now, then after that I'll try to get things sorted asap so things can get back to normal. |
Firebug has the disadvantage of being an add-on instead of Chromes console being integrated into the browser, and as a result it's a bit limited. Your 'var thing = [1,2,3];' will show as [1, 2, 3] in the Chrome console aswell, but that's just because it's the visual view. In both consoles, you can further inspect the element using something like 'var thing = { a: [1,2,3] };'. Both consoles will now show you an Object with a property a holding an array with length 3. In Chrome you can expand this object and view the extensive list of its properties, but in Firebug clicking it will send you to the DOM tab where you ONLY get to see the properties added by either the page or the console, so for an array that'd be only the array elements. I think I'll just do whatever changes I want to make on my current online version, and once you have a (hopefully stable) project ready just give me a sign and I'll re-fork. If there's anything that I have in the online version but isn't in yours then I'll just edit that in and send a commit back to you. |
Alright, working on it now. |
...it's arguing with me. May be a bit. :( |
Don't rush it, I can't look at it earlier than tomorrow. |
Alright. I believe I've got it all going with partial assignments working. For a small amount of code, it sure turned out to be pain. Also, I am going to go ahead and veto the tab before everything. I am sure it is not good practice, and just makes me have to scroll things to the right more. I've never seen anyone do that before. : / |
I had the need for a place to discuss smaller things without making issues for each.
The text was updated successfully, but these errors were encountered: