parkm / oldbpm

A JavaScript game where you shoot at bubbles.
0 stars 1 forks source link

Menus #28

Closed dgpt closed 10 years ago

dgpt commented 10 years ago

Create menus for Upgrades, Perks, Pausing, and Main.

parkm commented 10 years ago

Check out https://github.com/despondentdonkey/bpm/tree/menus. Tell me what you think.

dgpt commented 10 years ago

I'm having a really hard time understanding what's going on in State.pause. What is pauseState? Also, I'm not seeing the point of prevMenu and cachedState. We could test for instanceof Menu else if instanceof State.

I do really like putting the pause and restore methods in the State prototype, though. It's also a lot more concise than mine was.

Oh, one of the reasons I was doing the whole cacheState thing was so we could save states to cookies/cache easily. We could just save the state objects as JSON.

parkm commented 10 years ago

Yeah it's a little weird. So instead of having to go: state.pause(new PauseMenu(null, this)); you can just pass the menu like state.pause(PauseMenu) But you could also do the previous one just in case there's something different with the constructor.

Yeah while I was designing this I had the idea of a chain. But now that I think of it, if you pass a cachedState then it will just go to that and it will never have a chance of going to prevState. Trying to think if I forgot anything but yeah I may go ahead and change it to prevState.

Yeah that's what I was thinking of. But it's pretty dangerous to do such a thing. For example if we were to patch the game and change the state code around then set the state to some old cookie then that could cause some problems. It's much better to just save what needs to be saved. So what we can do instead is just have everything we want to save in an object then just put that in a cookie. So bpm.player

dgpt commented 10 years ago

Okay. Sounds good. The one thing that I'm still stuck on is restore.

        restore: function() {
            if (this.paused) {
                if (this.pauseState) {
                    setState(this);
                    current.init = true;
                    this.pauseState = null;
                }

                this.paused = false;
                this.onRestore();
            }
        },

Now, is there any possibility that this or this.pauseState will not be correct? This is tying my brain in a knot lol.

I think it's a good idea; it'll simplify menu creation.

Ah, good point. I didn't even think about that. In that case, I see no other point to the previous method.

parkm commented 10 years ago

Not sure what you mean. Are you wondering why I'm checking for the pause state? You can also use the pause and restore methods without specifying a pause state. It will just pause/unpause the state normally.

Yeah I learned that the hard way in my old GameMaker rpg. It was also the only built-in way to save.

parkm commented 10 years ago

Alright I removed prevMenu, cachedState, and replaced it with prevState. Good to merge?

dgpt commented 10 years ago

I dunno, I guess it's just confusing me a bit. I'll have to look at it again later.

Yep, good to go On May 23, 2014 11:41 PM, "Parker Miller" notifications@github.com wrote:

Alright I removed prevMenu, cachedState, and replaced it with prevState. Good to merge?

— Reply to this email directly or view it on GitHubhttps://github.com/despondentdonkey/bpm/issues/28#issuecomment-44078549 .

dgpt commented 10 years ago

(Low Priority) We should remove the window.addEventListeners from the Field state and replace it with our events.js listeners.

parkm commented 10 years ago

Is that even possible? events.js is for our custom events which we have to trigger on our own somewhere. We should just stick with the normal event listeners if we're dealing with DOM elements.

dgpt commented 10 years ago

Yeah it would be possible. I'm going to add a method events.global.triggerAll(name) which will trigger any event name attached to any object. Then we just add do something like this window.addEventListener('blur', events.global.triggerAll('blur')) This will give us the benefit of not having to explicitly remove the event listeners and making things a little cleaner.

parkm commented 10 years ago

I don't think it'd be worth the extra complexity. We shouldn't have that many window event listeners anyway. I think that onBlur and onFocus are really it.

dgpt commented 10 years ago

I don't think it's that much more complex, but alright.

parkm commented 10 years ago

The structure of menus are finished but they will need to be heavily polished during beta. So a new issue regarding that will be created in beta.