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

Update boardgame.io to 0.45.2 #105

Merged
merged 60 commits into from
Aug 11, 2021

Conversation

matthewejones
Copy link
Contributor

@matthewejones matthewejones commented Jul 21, 2021

Updated boardgame.io to version 0.45.2

Resolves #69

@matthewejones
Copy link
Contributor Author

Downloading files causes an internal server error:

TypeError: Cannot read property 'G' of undefined
      at getThreats (C:\Users\jonesm\Documents\elevation-of-privilege\src\server\publicApi.js:155:27)
      at C:\Users\jonesm\Documents\elevation-of-privilege\src\server\publicApi.js:128:25
      at async cors (C:\Users\jonesm\Documents\elevation-of-privilege\node_modules\@koa\cors\index.js:56:32)

@matthewejones
Copy link
Contributor Author

Turn order broken

@matthewejones
Copy link
Contributor Author

Turn order now works (I hope)

@ChristophNiehoff
Copy link
Collaborator

@matthewejones Do you know if PR #87 gets obsolete when this is merged?

@matthewejones
Copy link
Contributor Author

@matthewejones Do you know if PR #87 gets obsolete when this is merged?

Mongo DB shouldn't be required at all after this update. It just uses node-persist.

So it should be obsolete

@matthewejones
Copy link
Contributor Author

A bit more research into continuing to use MongoDB:

Using this I could probably set it up to continue using mongo or I guess we could just point anyone wanting to know in the right direction. Let me know what you think @ChristophNiehoff

@ChristophNiehoff
Copy link
Collaborator

I'd say, it's not our duty to implement a mongodb connector. Especially as we don't know if someone is actually using it. But I think we should document your findings in case someone wants to do it at some point. Maybe add a section to readme.md? Or add a dedicated file in which we write down such things?

README.md Outdated
As of boardgame.io v0.39.0, MongoDB is no longer supported as a database connector. There is currently no external library providing this functionality, however there is an [implementation](https://github.com/boardgameio/boardgame.io/issues/6#issuecomment-656144940) posted on github.


In order for the architectural model of the system to be saved to the database, the functions `setModel` and `fetch` will also need to be implemented. All the code relating to the database connector is in src/server/config.js
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we'll need more details here. Could you explain what exactly setModel and fetch are? Some functions from an interface? Which?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you also have to explain that don't only have to implement a connector but also implement an equivalent of ModelFlatFile?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've tried to add a bit more information, let me know if it's still a bit unclear.

src/game/eop.js Outdated
scores[ctx.playerID]++;
}

// TODO: have a cleaner or readable approach to updating this object
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do it! :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure of a cleaner way to do this, I might have a think about it and come back to it.

Copy link
Collaborator

@ChristophNiehoff ChristophNiehoff left a comment

Choose a reason for hiding this comment

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

@matthewejones I REALLY like how you cleaned up eop.js!!!

current = true;
}
const current = this.props.playerID === this.props.ctx.currentPlayer
// ? true : false to stop isInThreatStage taking undefined if this.props.ctx.activePlayers is undefined
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this comment still up-to-date?

@@ -41,7 +41,7 @@ class Leaderboard extends React.Component {
<td>
{hasPassed(idx, this) && <div align="center">&#10003;</div>}
</td>
<td><strong>{this.props.cards[idx]}</strong></td>
<td><strong>{this.props.cards[idx] ? this.props.cards[idx] : ''}</strong></td>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Syntactic sugar:
I believe you could wirte this using the nullish coalescing operator

<td><strong>{this.props.cards[idx] ?? ''}</strong></td>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just tried this, unfortunately it threw this error:

SyntaxError: C:\Users\jonesm\Documents\elevation-of-privilege\src\client\components\leaderboard\leaderboard.js: Support for the experimental syntax 'nullishCoalescingOperator' isn't currently enabled (44:52):

I can use || in this case even if I can't use ?? because cards will never be falsy but not null/undefined. So I think I'll just do this

@ChristophNiehoff ChristophNiehoff merged commit 9a9f58b into dehydr8:master Aug 11, 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.

Update to newer boardgame.io version
2 participants