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: first version of bananas-api #3

Merged
merged 2 commits into from Apr 14, 2020

Conversation

TrueBrain
Copy link
Member

@TrueBrain TrueBrain commented Mar 14, 2020

It serves both the webserver as "tusd", the resumable upload part
of the API.

This repository in combination with bananas-server allows you to
run your own BaNaNaS from scratch.

@TrueBrain TrueBrain force-pushed the first_version branch 4 times, most recently from b617cbb to f88f910 Compare March 15, 2020 16:45
@TrueBrain TrueBrain force-pushed the first_version branch 4 times, most recently from 50fd242 to 10cd299 Compare March 28, 2020 18:47
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

too much code to review in depth :p

Dockerfile Show resolved Hide resolved
content_api/index/common_disk.py Outdated Show resolved Hide resolved
content_api/index/local.py Outdated Show resolved Hide resolved
content_api/new_upload/exceptions.py Outdated Show resolved Hide resolved
content_api/new_upload/exceptions.py Outdated Show resolved Hide resolved
}

EXT_CTRL_CODES = {
0x01: (0, " "),
Copy link
Member

Choose a reason for hiding this comment

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

similar here - why some have a space and some not?

Copy link
Member

Choose a reason for hiding this comment

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

Some control codes resemble special whitespace or characters, while some resemble formatting (colour, font size).

(b[pos] & 0x07) << 18 | (b[pos + 1] & 0x3F) << 12 | (b[pos + 2] & 0x3F) << 6 | (b[pos + 3] & 0x3F),
)
else:
return (0, None)
Copy link
Member

Choose a reason for hiding this comment

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

Should error, rather than returning something?

content_api/new_upload/readers/scenario.py Outdated Show resolved Hide resolved
content_api/new_upload/readers/script.py Outdated Show resolved Hide resolved
content_api/new_upload/session.py Outdated Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the first_version branch 10 times, most recently from ba5957a to 7194b72 Compare April 5, 2020 12:59
@TrueBrain TrueBrain force-pushed the first_version branch 4 times, most recently from 52261a9 to 6d8e862 Compare April 11, 2020 09:19
@TrueBrain TrueBrain marked this pull request as ready for review April 11, 2020 09:19
@TrueBrain TrueBrain changed the title Add: first version of content-api Add: first version of bananas-api Apr 11, 2020
bananas_api/helpers/enums.py Show resolved Hide resolved
bananas_api/helpers/user_session.py Outdated Show resolved Hide resolved
bananas_api/new_upload/readers/cat.py Outdated Show resolved Hide resolved
bananas_api/user/github.py Outdated Show resolved Hide resolved
wishlist.txt Outdated Show resolved Hide resolved
wishlist.txt Show resolved Hide resolved
def validate_dependency(self, data, **kwargs):
if not DEPENDENCY_CHECK:
return

Copy link
Member

Choose a reason for hiding this comment

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

Validate content_type.

  • ai, ai-library: only allow ai-library
  • base-graphics, base-music, base-sounds: no dependencies allowed
  • game-script, game-script-library: only allow game-script-library
  • heightmap, newgrf: no dependencies allowed
  • scenario: only allow newgrf, ai and game-script

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this list is incompletely. Please correct me if I am wrong, but:

ai can also depend on scenario and game-script (for tutorial games)
game-script can also depend on scenario and ai (reverse of above)
newgrf can depend on other newgrfs

At least, that is what is in the current BaNaNaS. So I will implement that for now. If this is not what you expected, we put it on the wishlist to fix. As of course that means we first need to fix existing content :)

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 turns out to be a non-trivial addition. A version doesn't know what content-type it belongs to. In result, there is not a real way to check this in a consistent matter throughout the code. I need to think some more about this, but as such I am putting it on the wishlist for now, so we don't forget. Not sure it is mandatory for v1. If people abuse it, we can just kick them from the platform or something :D

Copy link
Member

Choose a reason for hiding this comment

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

When a scenario references a GS or AI, they are activated when loading the scenario.
But when you select a GS, this does not pick a single scenario. So, I think the dependency only makes sense in one direction.

About NewGRF depending on NewGRF, I guess this is along the line of "recommends". Fine to allow that.

bananas_api/helpers/api_schema.py Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the first_version branch 2 times, most recently from 81b68b2 to 82a1ed1 Compare April 12, 2020 08:44
.gitignore Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@TrueBrain TrueBrain force-pushed the first_version branch 4 times, most recently from becb998 to 30e27a4 Compare April 13, 2020 20:46
It serves both the webserver as "tusd", the resumable upload part
of the API.

This repository in combination with bananas-server allows you to
run your own BaNaNaS from scratch.
frosch123
frosch123 previously approved these changes Apr 14, 2020
The idea is that when-ever we find anything passing validation
where it shouldn't, we add an entry.

To make this somewhat sane, a custom regression-language was
created where you can target the API in a simple human-readable
way. The drawback is that it can be difficult to see what the
real calls are that are being made. This is a balance between
being able to motivate people to write regressions and being
very verbose in case the regression fails.
@TrueBrain TrueBrain merged commit 4c83d61 into OpenTTD:master Apr 14, 2020
@TrueBrain TrueBrain deleted the first_version branch April 14, 2020 18:10
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

3 participants