Skip to content

Dynamic rendering#41

Open
KorrAn34 wants to merge 15 commits into
alegian:0.13from
KorrAn34:dynamic-rendering
Open

Dynamic rendering#41
KorrAn34 wants to merge 15 commits into
alegian:0.13from
KorrAn34:dynamic-rendering

Conversation

@KorrAn34
Copy link
Copy Markdown

Please don't look at the files that were changed just jump in the game and look at the one story research entry thanks you won't regret it ok bye

Implemented rendering of research entry titles, paragraphs and figures (images with optional captions) so that one does not have to think of the page layout beforehand but rather just plop everything in there and it handles itself automatically.
Makes use of "PageFeatures" with three important Boolean properties
mustStartPage - this feature will cause a page break, starting on a new page
coversWholePage - if it is known that this feature is going to cover the whole page already during research entry creation, this is a small quality of life thing that makes it more straightforward to order entries
mustOccupySetPage - if set to true, this feature will always appear on a given page in the entry (represented by the next field in the constructor - preferredPageIndex). Must have sufficient content to cover pages preceding this and does not check whether features with the same preferred index all fit in the page, if they overspill outside the screen, so be it.

Also the code might be redundant in places and still contains some informative and debug snippets especially in DynamicRenderingHelper so please don't hate me uwu

Comment thread src/main/java/me/alegian/thavma/config/Config.kt Outdated
Comment thread src/main/java/me/alegian/thavma/impl/client/event/RegisterPageRenderersEvent.kt Outdated
Comment thread src/main/java/me/alegian/thavma/impl/client/gui/book/BookScreen.kt Outdated
Comment thread src/main/java/me/alegian/thavma/impl/client/gui/book/GridHelper.kt Outdated
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

it looks like you deleted content here

Comment thread src/main/java/me/alegian/thavma/impl/client/gui/book/EntryScreen.kt Outdated
}
}

fun relativeCenteredRenderable(renderable: Renderable, maxWidth: Int, texture: Texture) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

appears to only be used below, why not inline it

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

2d03f21 @KorrAn34 this is what i meant (resolve if agreed)

Comment on lines +23 to +24
private var maxWidth = BG.width/2 - 65
private var maxHeight = BG.height - 90
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

theres no point in passing maxwidth like this. the whole point of the layout system is to dynamically calculate/center all containers. didnt look in detail, but this can probably be achieved with a combination of containers/alignment options

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I am sorry I couldn't think of a better way. I am glad I somehow got the Rows(){} and Columns(){} to work.

Comment on lines +8 to +9
val mustOccupySetPage: Boolean
val preferredPageIndex: Int
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

mustOccupySetPage and preferredPageIndex i think are too complicated/hacky. i would remove them completely

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

My thought was if you e.g. want a crafting recipe to always be on the first doublepage on the right, you specify it easily. If the preceding text overspills due to a large font size (coming in in parallax) then it could be annoying to have to turn pages to find the recipe.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

thats solvable via extra features (eg bookmarks, shortcuts, previews, whatever). buggy spaghetti code is not solvable

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

because this should support other modders addons, and other modders wont understand this

event.register(PageFeatureTypes.PARAGRAPH.get(), ParagraphFeatureRenderer)
event.register(PageFeatureTypes.TITLE.get(), TitleFeatureRenderer)
event.register(PageFeatureTypes.FIGURE.get(), FigureFeatureRenderer)
event.register(PageFeatureTypes.FORMATTED.get(), FormattedTextFeatureRenderer)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

missing recipe

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

which by the way, we eventually need to add ways for modders to render their custom recipes if they want to

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

70d7a13

@KorrAn34 check my solution out and resolve conversation if youre okay

println("lineheight is $lineHeight")

// so that we get at least 2 lines at the end of the first page
val lineHeight = ceil((font.lineHeight * scale + 2))
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

whats the +2?

Comment thread src/main/java/me/alegian/thavma/impl/client/gui/layout/LayoutExtensions.kt Outdated

fun GuiGraphics.drawCenteredString(font: Font, text: Component, centerX: Float, color: Int = 0) {
drawString(font, text, (centerX - font.width(text) / 2f).toInt(), 0, color, false)
drawString(font, text, (centerX - font.width(text.visualOrderText) / 2f).toInt(), 0, color, false)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the point is to use the function below to reduce copy paste

Comment on lines +29 to +31
ComponentSerialization.CODEC.listOf().fieldOf("text").forGetter { _ ->
emptyList()
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

?? youre passing emptyList as getter???

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

read codecs and understand what forGetter is

Comment on lines 28 to 30
Codec.BOOL.optionalFieldOf("starts_page", false).forGetter(ParagraphFeature::mustStartPage),
Codec.BOOL.optionalFieldOf("has_set_page", false).forGetter(ParagraphFeature::mustOccupySetPage),
Codec.INT.optionalFieldOf("preferred_page", 1).forGetter(ParagraphFeature::preferredPageIndex)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

why are the string names different from the getters

Comment on lines +417 to +414
return { entryKey, titleIndex ->
return { _, _ ->
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

returning a lambda that ignores arguments makes no sense

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

also this func is unused? we need to test an example

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.

2 participants