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

Formspecs: Add container[] and container_end[] elements #4294

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
7 participants
@rubenwardy
Copy link
Member

rubenwardy commented Jul 7, 2016

#### `container[<X>,<Y>]`
* Start of a container block, moves all physical elements in the container by (X, Y)
* Must have matching container_end
* Containers can be nested, in which case the offsets are added
  (child containers are relative to parent containers)

#### `container_end[]`
* End of a container, following elements are no longer relative to this container

Can be used to make formspec themes -
themes can move the content down to make space for navigation etc

Containers are invisible - they are not rendered. They move content between the tags by the specified vector. They only exist during transition and parsing - the offset is applied when parsing the formspec, not at draw time

@rubenwardy rubenwardy force-pushed the rubenwardy:offset_fs branch to 3a6bc45 Jul 7, 2016

@rubenwardy rubenwardy changed the title Add offset formspec element Add container[] and container_end[] formspec elements Jul 7, 2016

@t4im

This comment has been minimized.

Copy link
Contributor

t4im commented Jul 7, 2016

Finally reusable formspec components, nice!

This can really change how we'll use formspecs, by allowing to hide complicated formspec compositions in functions and making them a lot more flexible without pages of string operations. 👍

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Jul 8, 2016

Isn't the container system already stack based?

@rubenwardy

This comment has been minimized.

Copy link
Member Author

rubenwardy commented Jul 8, 2016

Isn't the container system already stack based?

Forgot to alter the description, originally you used offset[x,y] to add to the current offset

@SmallJoker

View changes

src/guiFormSpecMenu.cpp Outdated
}

if (type == "container_end") {
parseContainerEnd(data);

This comment has been minimized.

Copy link
@SmallJoker

SmallJoker Jul 10, 2016

Member

return; again

@rubenwardy rubenwardy force-pushed the rubenwardy:offset_fs branch to 366b88f Jul 11, 2016

@rubenwardy

This comment has been minimized.

Copy link
Member Author

rubenwardy commented Jul 11, 2016

Fixed

@nerzhul

This comment has been minimized.

Copy link
Member

nerzhul commented Jul 13, 2016

You should also verify if container end are all closed at the end of the formspec

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Jul 13, 2016

If its not too much trouble, can you do the cleanup in a separate commit?

@rubenwardy

This comment has been minimized.

Copy link
Member Author

rubenwardy commented Aug 7, 2016

updated according to @nerzhul's suggestions

@est31: is there a command that easily allows you to do this?

@rubenwardy rubenwardy changed the title Add container[] and container_end[] formspec elements Formspecs: Add container[] and container_end[] elements Aug 7, 2016

@@ -278,6 +278,32 @@ void GUIFormSpecMenu::parseSize(parserData* data,std::string element)
errorstream<< "Invalid size element (" << parts.size() << "): '" << element << "'" << std::endl;
}

void GUIFormSpecMenu::parseContainer(parserData* data, std::string element)
{
std::vector<std::string> parts = split(element,',');

This comment has been minimized.

Copy link
@est31

est31 Aug 20, 2016

Contributor

space after comma

@@ -308,7 +334,7 @@ void GUIFormSpecMenu::parseList(parserData* data,std::string element)
else
loc.deSerialize(location);

v2s32 pos = padding + AbsoluteRect.UpperLeftCorner;
v2s32 pos = padding + AbsoluteRect.UpperLeftCorner + pos_offset * spacing.X;

This comment has been minimized.

Copy link
@est31

est31 Aug 20, 2016

Contributor

Don't multiply with the .X scalar of the spacing, but instead do the element wise schur product, notation is pos_offset * spacing. See also irrlicht source

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Aug 20, 2016

Otherwise it looks okay, but please next time make the cleanup (if its as large as here) in a separate commit.

@rubenwardy

This comment has been minimized.

Copy link
Member Author

rubenwardy commented Aug 25, 2016

Updated

@est31

This comment has been minimized.

Copy link
Contributor

est31 commented Aug 25, 2016

👍 when merging pls squash.

@est31 est31 added the One approval label Aug 25, 2016

@sofar

This comment has been minimized.

Copy link
Member

sofar commented Sep 7, 2016

Cursory glance on the code looks OK. I think this can be merged.

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Sep 21, 2016

I tried to merge but despite saying 'no conflicts' below 3 patches failed:

Applying: Add offset formspec element
error: patch failed: doc/lua_api.txt:1955
error: patch failed: src/guiFormSpecMenu.h:464

Applying: Containers instead of offsets
error: patch failed: doc/lua_api.txt:1412
error: patch failed: src/guiFormSpecMenu.cpp:278
error: patch failed: src/guiFormSpecMenu.h:373

Applying: Code style, spacing.X -> spacing
error: patch failed: src/guiFormSpecMenu.cpp:280

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Sep 23, 2016

@rubenwardy rubenwardy force-pushed the rubenwardy:offset_fs branch to c734f90 Oct 2, 2016

@rubenwardy

This comment has been minimized.

Copy link
Member Author

rubenwardy commented Oct 2, 2016

rebased

@rubenwardy

This comment has been minimized.

Copy link
Member Author

rubenwardy commented Oct 2, 2016

👍 I'm pretty confident that this is stable

@paramat

This comment has been minimized.

Copy link
Member

paramat commented Oct 3, 2016

@paramat paramat closed this Oct 3, 2016

@rubenwardy rubenwardy deleted the rubenwardy:offset_fs branch Dec 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.