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

Add Two Phase Updates feature #273

Merged
merged 13 commits into from
Jul 14, 2019
Merged

Add Two Phase Updates feature #273

merged 13 commits into from
Jul 14, 2019

Conversation

AstraLuma
Copy link
Member

@AstraLuma AstraLuma commented May 18, 2019

Felt like writing a two-phase update system.

This is so much easier to use with #263 that it's a soft requirement.

This assumes that events signaled are queued and handled after all of the current event handlers are called.

This skipped our usual discussion phase of things, where we debate designs first. Given that, feel free to completely rip this apart.

@AstraLuma AstraLuma requested a review from a team as a code owner May 18, 2019 02:10
@AstraLuma
Copy link
Member Author

This is now a lot more reasonable.

Copy link
Collaborator

@pathunstrom pathunstrom left a comment

Choose a reason for hiding this comment

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

The only comment I have for this is that right now, you can't have complex multiple interactions with this two phase commit system since it puts the commit immediately on top of the update event, and nothing can happen between them.

If that's intentional, it's a good sample of how to set up something like this, so don't feel that's a failing.

Only other comment attached, let me know and I can merge this.

if __name__ == "__main__":
logging.basicConfig(level=logging.INFO)

# Not good practice, https://github.com/ppb/pursuedpybear/issues/263
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can fix this now.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually not, unless we include the entire kwargs block from __init__.py.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is improvable with #307

Copy link
Collaborator

Choose a reason for hiding this comment

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

#307 is merged, we can actually fix this now.

@AstraLuma
Copy link
Member Author

That was pretty much the idea? Everyone runs their simulations during Update, but stages the changes, so everyone sees a consistent view of the application state.

The simplest usage would be Conway's Game of Life, written such that each cell performs its own logic. Each cell would run its simulation during the Update phase and stage the changes, which would be applied during the Commit phase, after all other cells have run their simulation.

It did cross my mind to extend this to allow arbitrary phases, so you could have, for example, AI, Physics, and Commit phases. But this seemed like a good, solid start.

Copy link
Collaborator

@pathunstrom pathunstrom left a comment

Choose a reason for hiding this comment

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

Holding until we fix the example.

Copy link
Collaborator

@pathunstrom pathunstrom left a comment

Choose a reason for hiding this comment

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

Looks good to me.

bors r+

bors bot added a commit that referenced this pull request Jul 14, 2019
273: Add Two Phase Updates feature r=pathunstrom a=astronouth7303

Felt like writing a two-phase update system.

This is so much easier to use with #263 that it's a soft requirement.

This assumes that events signaled are queued and handled after all of the current event handlers are called.

This skipped our usual discussion phase of things, where we debate designs first. Given that, feel free to completely rip this apart.

Co-authored-by: Jamie Bliss <jamie@ivyleav.es>
@pathunstrom pathunstrom added the bors Someone has bors r+ this PR label Jul 14, 2019
@bors
Copy link
Contributor

bors bot commented Jul 14, 2019

Build succeeded

  • docs
  • Linux python:3.6-slim
  • Linux python:3.7-slim
  • macOS PYTHON:3.6.8
  • macOS PYTHON:3.7.2
  • pep517
  • Windows python:3.6-windowsservercore-1809
  • Windows python:3.7-windowsservercore-1809

@bors bors bot merged commit c4dc2d7 into ppb:master Jul 14, 2019
@AstraLuma AstraLuma deleted the feature-twophase branch July 14, 2019 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bors Someone has bors r+ this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants