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

spatial flux rework #3600

Merged
merged 13 commits into from Nov 14, 2021
Merged

Conversation

agrrr3
Copy link
Contributor

@agrrr3 agrrr3 commented Nov 4, 2021

discussion:
https://freeorion.org/forum/viewtopic.php?p=104863

missing:
graphics for the composite are missing (and the lance could be better), those are already requested on the forums

https://freeorion.org/forum/viewtopic.php?f=10&t=12175&p=108300
https://freeorion.org/forum/viewtopic.php?f=10&t=12176

@geoffthemedio geoffthemedio added category:refactoring The Issue/PR describes or contains an improved implementation. component:content scripting The Issue/PR deals with the FOCS language, turn events or the universe generator. labels Nov 4, 2021
@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 4, 2021

I am not able to interpret the CI failures... TestPythonParser (Failed) - as far as I can see this PR should not be able to break anything there

@geoffthemedio
Copy link
Member

TestPythonParser (Failed) - as far as I can see this PR should not be able to break anything there

I'm not sure, as @o01eg set up those tests, but it looks like the Python parser tests also run the existing C++ / Boost tests on the .focs.txt files, and checks the resulting number of techs. Since you're adding more techs, the numbers don't match what it expects.

@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 4, 2021

tests on the .focs.txt files, and checks the resulting number of techs. Since you're adding more techs, the numbers don't match what it expects.

thanks, missed that part of the test. guess the test is brittle though

@geoffthemedio
Copy link
Member

I think the purpose is to let him check that the Python and Boost parsers produce the same results, and that specific test is perhaps just there as a sanity check or warning that something was changed so that whatever Python scripts are being parsed might need updating?

@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 4, 2021

reworked the stealth effect and fixed the lance damage (forgot it does only half the damage as it is a close range weapon)

@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 5, 2021

playtested this, good to go (after review)

@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 9, 2021

@o01eg @geoffthemedio can we merge, please

@o01eg o01eg added this to the v0.5 milestone Nov 9, 2021
@geoffthemedio
Copy link
Member

I don't have time to look at this until later this week

default/stringtables/en.txt Outdated Show resolved Hide resolved
default/stringtables/en.txt Outdated Show resolved Hide resolved
@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 11, 2021

addressed the review comments and did some further fixes on the descriptions; the hull descriptions were not up to date with the values.

@agrrr3
Copy link
Contributor Author

agrrr3 commented Nov 11, 2021

Oh, and should I commit the gimp file i used to compose the flux-lance.png to the asset repository (maybe at art/icons/ship_parts/
?). Artistic depth is not high, i fiddled with the texture and colors in death_spore.blend and composed it on top of the death_spore.png (and added a color change layer)
As far as I can see the compositions of renders are not committed there currently.

@geoffthemedio
Copy link
Member

If it's just overlaying existing available textures and tweaking colours, it's probably not super useful to archive the intermediate file format, but I'm not sure about that sort of thing.

default/stringtables/en.txt Outdated Show resolved Hide resolved
default/stringtables/en.txt Outdated Show resolved Hide resolved
agrrr3 and others added 3 commits November 13, 2021 14:31
Co-authored-by: Geoff <geoffthemedio@users.noreply.github.com>
Co-authored-by: Geoff <geoffthemedio@users.noreply.github.com>
Co-authored-by: Geoff <geoffthemedio@users.noreply.github.com>
@geoffthemedio geoffthemedio merged commit ed79c4c into freeorion:master Nov 14, 2021
@geoffthemedio geoffthemedio added the status:merged All relevant commits of this PR were merged into the master development branch. label Nov 14, 2021
@Vezzra
Copy link
Member

Vezzra commented Nov 16, 2021

@agrrr3,

Oh, and should I commit the gimp file i used to compose the flux-lance.png to the asset repository (maybe at art/icons/ship_parts/

Yes, please.

As far as I can see the compositions of renders are not committed there currently.

There are a lot of images which don't have corresponding source files in the assets repo, because the assets repo was introduced only in 2014 (over a decade after the project started), and by that time we only had been able to recover the source files of a couple of the artwork in FO. Most of them we don't have.

Even after that we often/sometimes forgot to ask for the source files of artwork various people contributed, so unfortunately the assets repo is quite incomplete.

But that does not mean we still try to get every source file in we can... 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:refactoring The Issue/PR describes or contains an improved implementation. component:content scripting The Issue/PR deals with the FOCS language, turn events or the universe generator. status:merged All relevant commits of this PR were merged into the master development branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants