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

Process Organelles #57

Merged
merged 1 commit into from Nov 20, 2013
Merged

Process Organelles #57

merged 1 commit into from Nov 20, 2013

Conversation

bkloster
Copy link
Contributor

See #41. Work in progress, don't merge yet.

@bkloster
Copy link
Contributor Author

I took the liberty of fixing some minor errors. The "ProcessOrganelle is nil" error you got actually came from a syntax error in the file (Lua has no -= operator). It starts error-free now, but still doesn't seem to work. I'll leave that to you.

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

Everything should be good! Can you confirm that there are no random tabstops and everything looks good to you?

@bkloster
Copy link
Contributor Author

No tab stops there (did you find something in your setup?).

The processing seems to work, but the speed of conversion is frame-rate dependent. To fix this, it's probably necessary to not just limit the frequency of conversion, but also how fast the resources are stored inside the ProcessOrganelles. If that's not limited, the ProcessOrganelles will keep filling up and suck the vacuoles dry.

As a side note, I was confused by invisible agent storage. I parked the microbe besides the oxygen emitter and watched the agent count in the HUD. Since I never saw oxygen go above zero, I assumed that the ProcessOrganelle converted 1 oxygen into 1 oxytoxy although it was configured for a 2:1 rate. So we either need to show the player how much the ProcessOrganelles have stored, or switch to a non-buffered processing.

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

Just found out that save/load game functionality actually was implemented (and my code here had a bug with it) I'll get to fix that.

To fix this, it's probably necessary to not just limit the frequency of conversion, but also how fast the resources are stored inside the ProcessOrganelles.

  • What about a cooldown on absorbing for process organelles. The ProcessOrganelle:takeAgent would be called on the 'lucky' process organelle for oxygen, it would then return false (or the non-taken amount) if the organelle is still on its absorb-cooldown and noone would get any oxygen that frame.
    This would still, however, result in the first oxygen you absorb after a long period of not absorbing oxygen disappear instantly (perhaps still too confusing) after which it would start building up oxygen in storage.
  • Alternatively a cooldown on how often the microbe lets anyone absorb anything, this wouldn't allow for different cooldowns on each storage organelle tho, unless combined with the previous solution.

The two solutions could make the displayed agents more intuitive, but if you don't think that'll be enough, it is not impossible to display the sum of oxygen (in vacuoles and buffers) instead of only the oxygen in vacuoles.

Edit: Regarding tabstops, i have no clue. I understand my first pull request had tabstops, but for the second one i'm baffled. Perhaps it's codeblocks that adds them while pretending to add spaces. (I've exclusively used Notepad++ for this branch)

@bkloster
Copy link
Contributor Author

I'd be partial to the first option, as it would allow designers to tweak the conversion rate by adjusting the ProcessOrganelle's cooldown. Ideally, ProcessOrganelle would have a property "conversionInterval" that gives the time between two conversions. The organelle would figure out by itself how much of each input agent it must take in each frame to maintain that interval in times of abundant resources.

The confusion I mentioned could also be handled in the organelle graphics. Gradually changing the color of the organelle (or, when we have such a thing, adjust some animation) as it fills up will not only give a sense of "where do my resources go", but also "when will that organelle do something" (in case the conversion interval is several seconds long).

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

Good ideas! I'll try it out later

@@ -10,6 +10,7 @@ function MicrobeComponent:__init()
Component.__init(self)
self.organelles = {}
self.vacuoles = {}
self.producerOrganelles = {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more consistent, shouldn't this be called processOrganelles?

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

I think you'll like the cooldown system i just pushed. Only missing changing color of ProcessOrganelle relative to buffer storage now.

@bkloster
Copy link
Contributor Author

One of the locals wandered off. I put it back where it belongs.

Now, one question about the cooldown. What does timeBetweenProcess do? It seems to only ever be assigned the same value of processCooldown.

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

Now, one question about the cooldown. What does timeBetweenProcess do? It seems to only ever be assigned the same value of processCooldown.

timeBetweenProcess as the name suggest is the minimum time between producing output. processCooldown is set to timeBetweenProcess when something is produced and then counts down until it reaches 0, once it reaches 0 another output can be produced. You missed this line in update:
self.processCooldown = self.processCooldown - milliseconds

I'm currently struggeling with lua arithmetic making the colors change. Sloowly making progress.

@bkloster
Copy link
Contributor Author

Ah, now I see. May I suggest to rename timeBetweenProcess to processCooldown and processCooldown to cooldownLeft? It would make the difference between the two more obvious, I think.

By the way, I thought of another model to restrict the conversion speed. Instead of waiting for the cooldown, then consuming input agents from its buffer, the ProcessOrganelle fills its buffer at a rate that will allow one conversion every interval seconds.

For example, the interval is 2 seconds and the organelle requires 5 oxygen and 3 nitrogen. This means it needs 2.5 oxygen per second and 1.5 nitrogen per second to maintain its intended conversion rate. During a timestep of 16 milliseconds, the organelle will try and grab 2.5 * 0.016 = 0.04 oxygen and 1.5 * 0.016 = 0.024 nitrogen for its buffer. As soon as the buffer contains enough, its contents are consumed and the output is produced.

This would basically mean that a process organelle requires an abundance of resources over the duration of its interval to function at max capacity, which is an easy rule to remember for players. If resources are scarce and organelles have to fight over them, they will work slower. Furthermore, if the player gathers up resources, they are consumed at a steady rate instead of being gone in an instance because the processors' internal cooldowns are all elapsed. And probably the most interesting advantage is that it's easy to adjust the effectiveness of ProcessOrganelles on the fly - just increase or decrease the interval. That can be a nice target for toxins.

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

rename timeBetweenProcess

I only named it that because you seemed to deem that less confusing in a comment above.

By the way, I thought of another model to restrict the conversion speed

Thats exactly how i have implemented it xD except i don't absorb fractions, but wait until enough time have elapsed to absorb 1 unit.

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

There you go! crazy colours and all the fixes you could dream of. Changed the oxygen amount required for a oxytoxy to 4 to better display the colour change.

I was very confused for a long time by a bug i discovered. The oxygen emitter will instantly when the game start, emit 2 oxygen units somewhere on top of / below the player microbe (even tho the emitter is far away from the player) which the microbe will then instantly absorb.
You'll see the microbes process organelle be non-grey (process organelle should be grey when its buffer is empty) due to this. If you disable distribution of oxygen to processorganelles you will notice that 2 oxygen are stored from the start. You can verify that it actually is the emitter as if you remove the code that generates the emitter in setup.lua, the two oxygen wont appear.

@bkloster
Copy link
Contributor Author

Thats exactly how i have implemented it

Oh, I see now. Hm, well not exactly. In fact, there's still a rather serious problem in your implementation, unless I'm missing something (again). If I read this line right, a ProcessOrganelle can take at most 1 of each resource per frame. What if we have a crazy organelle that needs 100 oxygen, and a very short cooldown (say, 1 millisecond)? At a framerate of 60 Hz, the actual cooldown would be 100 frames or about 167 milliseconds. At 20 Hz, it would still be 100 frames, but 500 milliseconds.

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

I'll fix it, but it will probably be a small contribution towards the 'spiral of death' (http://gafferongames.com/game-physics/fix-your-timestep/)

@jjonj
Copy link
Contributor

jjonj commented Oct 31, 2013

There's my best bid on a solution. It distributes 1 unit of each agent type (at max) per 100 milliseconds (adjustable).

Edit: I'll merge when i have your final approval

@ghost ghost assigned bkloster Nov 3, 2013
@bkloster
Copy link
Contributor Author

bkloster commented Nov 4, 2013

Oops, sorry. I thought I already posted on this. Must have closed the browser tab before clicking the "comment" button.

Honestly, I'm not really happy with that solution. For one, I think that while loop isn't valid Lua syntax, looks more like C++ (don't worry, I only just noticed that too after looking at it for a combined 15 minutes at least). More importantly, unless I'm mistaken, this implementation only allows for 10 agents per second per vacuole to be distributed to all other organelles that want it, right? So if we have one vacuole and two process organelles that want to consume 5 agents per second each, that's fine, but as soon as the player adds another process organelle, the conversion speed of all three process organelles goes down.

I'm beginning to think that we might need to reevaluate the way that agents are distributed inside a microbe. Especially because this distribution is not specific to process organelles. Almost organelles will consume some agent (mostly energy, but maybe also other stuff). I'll think about it for a bit and probably make a forum post (or a GitHub ticket) to discuss it.

@jjonj
Copy link
Contributor

jjonj commented Nov 4, 2013

looks more like C++

Wow that was dumb, can't believe i forgot to actually test it.

this implementation only allows for 10 agents per second per vacuole to be distributed to all other organelles that want it, right?

Personally i saw this more as a feature than a problem, but yeah lets think & discuss!

@bkloster
Copy link
Contributor Author

bkloster commented Nov 4, 2013

Don't get me wrong, a feature like that is probably a good idea. I just feel that it should be more explicit and configurable (per-vacuole setting for "agent bandwidth") than this.

@jjonj
Copy link
Contributor

jjonj commented Nov 4, 2013

Yeah, you're right!

@jjonj
Copy link
Contributor

jjonj commented Nov 10, 2013

After reading through the discussion on the forum again and some consideration, i feel this is sufficient for now.

  • Regarding the limit of 1 unit per 100 milliseconds, the designers seem to want it to be dependant on the size of the microbe (number of hexes), which we don't have a way of querying right now.
  • The idea of a transport weight will be postponed, as discussed.
  • The distribution of scarce resources: We don't realy have a good solution yet, and i personally feel the currently implemented one does a fine job and any of the issues faced right now won't come into play until we make really complex cells anyway.
  • Everything else that was discussed on the forum was relating to vacuoles and storage in general, which should be changed in a seperate issue.

@bkloster
Copy link
Contributor Author

Could you add some documentation for the processing and agent distribution algorithm? Especially the detail that at most 1 of each agent is distributed every 100 milliseconds. And maybe refactor that hardcoded 100 ms into a constant AGENT_DISTRIBUTION_INTERVAL or something like that.

@bkloster
Copy link
Contributor Author

Can you rebase this onto master and squash the commits, as described in the style guide? I tried, but there were merge conflicts that I wasn't sure how to resolve without breaking your code.

@jjonj
Copy link
Contributor

jjonj commented Nov 13, 2013

Might aswell wait for game states to be merged, I'll do it later .)

@bkloster
Copy link
Contributor Author

The game state branch is merged now.

@jjonj
Copy link
Contributor

jjonj commented Nov 13, 2013

That was a bit complex, but it looks like it worked. Altough i had to fix... something. for it to be willing to merge.

@bkloster
Copy link
Contributor Author

That doesn't look right. The result should be one single commit, adding only the stuff that has been changed in this particular branch. Right now, there are two commits in this branch, one of them absolutely huge, containing changes that already are in master (the style guide, for example).

Hold on, I'll see if I can fix this.

@bkloster
Copy link
Contributor Author

There you go. Can you give it a final once-over to make sure I didn't break anything? If everything's fine, go ahead and merge it.

As for what happened with your rebase attempt, I suspect it had to do with the intermittent rebases / merges that were done to pull in changes from master into this branch. I removed all commits that weren't related to process organelles (i.e. those that were pulled in from master) with an interactive rebase, then rebased again onto the current master.

@jjonj
Copy link
Contributor

jjonj commented Nov 14, 2013

Thanks for fixing it
I did merge with master before i did the rebase, which also makes solving conflicts easier, i guess that must have been the problem.
You did remove some commits that you shouldn't have, regarding changing compounds to atp, oxygen and glucose, I'll fix it up later. I merged your lua::error changes aswell, so i'll have to rebase on to master again aswell for those two things, hopefully doing it right this time.

@jjonj
Copy link
Contributor

jjonj commented Nov 14, 2013

I fixed it up and rebased again.
You'll see a lot of changes in setup.lua because i also fixed up related variable names a bit.
Feel free to merge.

Closes #41

squashed commits:
Add ProcessOrganelle for converting compounds

Closes #41

Add ProcessOrganelle

Fixed syntax in process_organelle.lua and added hasInputAgent method to ProcessOrganelle class

Added ProcessOrganelle support into Microbe class, and added process_organelle.lua to manifest

Added code to setup.lua to test ProcessOrganelle (currently not working)
Minor fixes to ProcessOrganelle related code in process_organelle.lua and microbe.lua

Minor formatting and reintroduced function missing in process_organelle.lua

Fix some minor errors in the ProcessOrganelle script

* Use table.insert instead of a[#a+1] to not repeat the table name
* Call superclass' constructor in ProcessOrganelle.__init
* Explicitly pass self to Organelle.update in ProcessOrganelle.update
* Lua has no -= (or += for that matter)
* Typos

Fixed error in loading a savegame with ProcessOrganelle.

Removed debugging code in microbe.lua

Created cooldown system for Process organelles, limiting the frequency of agent absorbtion and production

Fixed missing local specifier on two variables in process_organelle.lua

Implemeneted colours for process organelles to reflect how filled it is. Also minor bugfixes

Move "local" to where it belongs

Made distribution of agents, dependant on time elapsed instead of frames. (1 agent per agent type distributed per 100ms)

Release candidate 1 Done.

- Fixed compile error
- Removed debugging code
- Changed setup of compounds/agents to reflect mitochondrion

Added additional documentation for processing compounds.
Minor refactoring to compound distribution for more accuracy.

Replaced magic value with proper constant

Re-changed emitters to use new oxygen, glucose, atp, co2 system.
Cleanup of old variable and comment names in setup.lua
bkloster added a commit that referenced this pull request Nov 20, 2013
@bkloster bkloster merged commit ee99cfd into master Nov 20, 2013
@bkloster bkloster deleted the 41-processing-organelles branch November 20, 2013 07:54
hhyyrylainen added a commit that referenced this pull request May 4, 2021
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

Successfully merging this pull request may close these issues.

None yet

2 participants