Add Layouts and renderFacade task - #200
Conversation
[Maven Build Status]📑 Commit: 📦 Download artifact: Generator.jar |
0614713 to
645c300
Compare
346456e to
d7ca222
Compare
renderFacade task
|
@pyrollo I focused mainly on removing dead code, resolving or completing TODOs, and improving the algorithm used by concatenate (on Also I have added multiple unit tests as I have discovered some bugs on For the algorithm, in a nutshell, I have used I reused your approach and added a second phase for starved builders. This fixes 2 issues where space where not distributed evenly. This work is now ready for a PR, and the documentation was done. It is no longer a work in progress. It is a bit unusual to open a PR at the end, hope you'll be able to take it from here (Although not a good time). |
d7ca222 to
ba339a2
Compare
indyteo
left a comment
There was a problem hiding this comment.
Il y a un "gros" rebase à faire aussi (cette branche a 15 commits de retard sur le main).
Bon au final ça fait énormément de commentaires mais une écrasante majorité est sur des fautes d'anglais, de frappe, ou de copié/collé ou renommage perdu en route... Donc il ne faut pas s'affoler, même si ça va encore probablement casser GitHub de faire autant de commentaires dans une même review 😅
Si jamais une simplification de code que j'ai proposé semble ne pas valoir le coup, demander trop de travail, ou ne pas fonctionner parce que j'avais oublié un détail, ne pas hésiter à juste ne pas la faire, on pourra toujours revenir dessus plus tard si ça s'avère nécessaire / pertinent.
Il y a notamment à plusieurs reprises des remarques sur des champs présents à l'identique dans toutes les implémentations de certaines interfaces. On peut tout à fait décider de laisser en l'état et de s'en occuper plus tard... ou pas.
Au final, il n'y a qu'une seule "grosse" remarque (enfin qui risque de demander vraiment du temps), c'est l'ajout d'exemples de layouts avec des illustrations dans le documentation. Pareillement, ça peut se faire dans une autre PR après, par exemple avec l'ajout des how-tos, ou une fois que le paramétrage aura été retravaillé et sera un peu plus définitif. Dans ce cas ne pas en tenir compte pour cette PR !
feb1273 to
8c04233
Compare
|
Merci pour la relecture et d'avoir repris la PR : ) J'ai regardé à peu près tous les commentaires, mais comme GitHub est cassé, je mets les liens de mes commentaires:
Edit: J'avais pas vu le Delegate |
bfdfd52 to
df2af65
Compare
df2af65 to
2b3c5ff
Compare
indyteo
left a comment
There was a problem hiding this comment.
Dernière passe avant de pouvoir merger, les dernières typos et incohérences après renommages.
Le coup de l'AxisParams à fusionner c'est juste parce que je m'en suis rendu compte en relisant qu'on avait déjà un truc similaire ailleurs, mais si ça pose le moindre soucis de n'en garder qu'un, laisse tomber la remarque
4abdc6b to
1af5f1e
Compare
Co-authored-by: Pierre-Yves Rollo <dev@pyrollo.com>
Co-authored-by: Pierre-Yves Rollo <dev@pyrollo.com>
1af5f1e to
1f67546
Compare
indyteo
left a comment
There was a problem hiding this comment.
Et une bonne chose de faite !
Changes
This PR adds layouts and a task to render facades.
Feature description
Layouts are a kind of structure that can resize itself to fit a requested space. Contrary to Structure, its size aren't fixed, it can stretch, be repeated to fill the available space.
With that we can define layouts and have building facades rendered dynamically.
The type of Layouts are:
Showcase
Reason
This adds more expressiveness on building rendering. Moreover with models values, we can have layout that are rendered depending on metadata, randomly..
TODOs
Left one TODO on
full.yaml. Layout parameters can be very long, it could a good idea to define it elsewhere. (As it uses voxels defined on format, could becommon.yamlfile. But that is a detail.Self-checks
/docsfolder has been updatedexamples/work the same (or have been adapted if subject to changes in this PR)TODOs