Secretchronicles / TSC

An open source two-dimensional platform game.
https://secretchronicles.org/
GNU General Public License v3.0
204 stars 50 forks source link

Allow tsc to be built using external mruby #447

Open muammar opened 9 years ago

muammar commented 9 years ago

In debian, there exists already a mruby package:

 ✘ muammar@zarathustra ⮀ /tmp/tsc ⮀ aptitude show mruby 
Package: mruby                           
State: installed
Automatically installed: no
Version: 1.1.0+20150601+gitbd2686d8-1
Priority: extra
Section: ruby
Maintainer: Nobuhiro Iwamatsu <iwamatsu@debian.org>
Architecture: amd64
Uncompressed Size: 2,349 k
Depends: libc6 (>= 2.14)
Conflicts: mruby
Description: lightweight implementation of the Ruby language
 mruby is the lightweight implementation of the Ruby language complying to the ISO standard. This can be linked and embedded within your
 application. 

 This package contains the following tools: 
 * mirb: Embeddable interactive ruby shell 
 * mrbc: mruby compiler 
 * mruby: mruby interpreter 
 * mrdb: mruby debugger 
 * mruby-strip: mruby strip

I understand the motivation on having mruby compiled statically, but in order to upload tsc in debian, the src has to be able to compile using the mruby debian package.

Thanks.

Quintus commented 9 years ago

@Luiji @datahead8888 Do you think this should go into the 2.0.0 release (causing RC2 by classifying it as a critical bug)? Or should it only be applied to devel for the next version?

Valete, Quintus

Quintus commented 9 years ago

@muammar mruby can be built in a lot of different configurations, especially not all features it provides are applicable to every project. If TSC is compiled with an mruby that has features enabled which are not enabled in the version we ship, we can’t support the resulting executable. So unless Debian has a wealth of different mruby packages in its reposotories, all with different build configurations, there’s a major problem here. For the moment, we only use mruby’s default configuration, but it doesn’t have to stay like that.

Also we compile some 3rd-party mruby extensions (GEMs) into the mruby library we use, see this directory. These are directly part of the mruby library when compiled and cannot be shipped on their own. So by compiling TSC against a differently configured mruby will result in features missing that we officially support.

Suggestions?

Valete, Quintus

muammar commented 9 years ago

@Quintus I understand the point on compiling an own version. I think I have to send a mail to the mailing list (maybe in debian mentors), to see what can be done in this situation.

datahead8888 commented 9 years ago

It sounds like muamar's suggestion to follow up with the Debian people is a logical choice. If we're forced to change something to make the game publishable in Debian (and are able to do so), I would agree we have to call this a critical bug and make a new release candidate. Per Quintus's analysis above, though, it sounds like it might be a bit of a mess to try to build mruby differently. @Luiji might have some good insight into this decision, since he has more Ruby experience than me.

@muammar - I understand that we also compile CEGUI statically. Will this cause issues in Debian at all?

muammar commented 9 years ago

@datahead8888 I think that tsc should also be capable of being compiled with the cegui libraries available in the debian archive. I am sure of that. With mruby is different, because there is also a debian policy about ruby packages (that I have to read).

For the moments, I can clone the repo and make a pull request in github adding the debian directory needed to make debian packages while this is solved.

What do you think?

Quintus commented 9 years ago

I understand that we also compile CEGUI statically. Will this cause issues in Debian at all?

This isn’t true. On @Akien’s request, this was made optional. CEGUI is only automatically downloaded and compiled in statically if it is not found on the compilation system.

If we're forced to change something to make the game publishable in Debian (and are able to do so), I would agree we have to call this a critical bug and make a new release candidate. Per Quintus's analysis above, though, it sounds like it might be a bit of a mess to try to build mruby differently

We even have a hard dependency on the json GEM for mruby, because it gets used for serialising and restoring the data a level script writer saves in the save event of a level. Without this, the save and load event wouldn’t work. If it is missing, it even is likely that on savegame creation or loading the mruby interpreter crashes, which causes unpredictable behaviour.

A workaround may be to finally make our ENABLE_MRUBY switch working properly (currently it is pretty useless) and allow it to completely disable scripting support in TSC. Users are going to miss a major feature with that, though. Also that’d require so many code changes that it would not go into 2.0.0, but in a later version.

because there is also a debian policy about ruby packages (that I have to read).

Beware that mruby is not ruby. It has nothing to do with the canonical Ruby interpreter (apart from that its main developer is the inventor of the Ruby programming language). mruby is more like Lua in its goals, implementation, and intention, if you need something to compare.

For the moments, I can clone the repo and make a pull request in github adding the debian directory needed to make debian packages while this is solved.

Pull requests are fine. If you don’t want to do it in GitHub, that’s also fine. Just give us something we can git clone.

Valete, Quintus

Quintus commented 9 years ago

Indeed it’d be nice to have @Akien in this discussion as he’s the packager for Mageia. @Akien, how did you cope with the mruby dependency?

Vale, Quintus

muammar commented 9 years ago

I understand that we also compile CEGUI statically. Will this cause issues in Debian at all?

This isn’t true. On @Akien’s request, this was made optional. CEGUI is only automatically downloaded and compiled in statically if it is not found on the compilation system.

@Quintus thanks for clarifying this.

because there is also a debian policy about ruby packages (that I have to read).

Beware that mruby is not ruby. It has nothing to do with the canonical Ruby interpreter (apart from that its main developer is the inventor of the Ruby programming language). mruby is more like Lua in its goals, implementation, and intention, if you need something to compare.

I had no idea. Thanks for the information. In any case, I have asked what to do in the debian mailing list.

For the moments, I can clone the repo and make a pull request in github adding the debian directory needed to make debian packages while this is solved.

Pull requests are fine. If you don’t want to do it in GitHub, that’s also fine. Just give us something we can git clone.

It is ok for me to do a pull request. I will work on that.

I am interested to know how @Akien tackle this in Mageia.

muammar commented 9 years ago

Getting back to this. I got a reply in the debian mailing list. In summary, changes made to mruby has to be included either in upstream or in the debian distributed version (by submitting a bug report in the debian tracking system to request changes to the maintainers). Regarding the extensions, they have to be packaged (I can do this). What I would need to know it is how many there are.

It is also needed to avoid statically linked libraries. I read in the tsc mailing list about how to pass to cmake for example the path to cegui.

Quintus commented 9 years ago

In summary, changes made to mruby has to be included either in upstream

This won’t happen due to mruby’s policy, which is simplified this: Keep the core super-small, and have everything else be an unsupported (= community-supported) 3rd-party extension. Things like the JSON extension thus will never go into upstream mruby. I don’t have too much experience in Lua, but I’d expect that Lua’s core API is not exendable in the way the mruby core API is.

The procedure of integrating an mruby installation works like this:

  1. Download mruby source code
  2. Download 3rd party extension (GEM)
  3. Tell the mruby build script about the 3rd party GEM
  4. The mruby build script compiles a single static library consisting of both the mruby core code and the 3rd party extension.
  5. You link that static library into your project.

That is the officially endorsed mechanism for mruby and the entire build system of mruby is centered around this, and it is the reason why we ship with our own build script for mruby — this is the way mruby is intended to be integrated.

There are dozens of 3rd party GEMs in the wild. Building mruby with every combination of them is likely to be impossible. I suggest you file a ticket on mruby’s repository so you can discuss the problem directly with the mruby upstream devs. You might also want to ask the maintainer of the mruby Debian package.

To clarify as it appears from the message you quoted that this was misunderstood: We did not fork and extend mruby. We used its official plugin mechanism. This makes quite a difference.

Regarding the extensions, they have to be packaged (I can do this)

It follows from the above that this is not possible due to technical reasons (there will only ever be one mruby library built from the core + the GEMs). Maybe I’m wrong with this, though. Better ask upstream at the mruby repository.

What I would need to know it is how many there are.

This is rather simple. Here’s the list of GEMs we use: https://github.com/Secretchronicles/TSC/tree/v2.0.0-rc1/mruby/mgems

If you ask upstream, please link in the issue on the mruby repository here so we can follow the discussion there.

Vale, Quintus

Quintus commented 9 years ago

@muammar we are two days in front of the final release. If you tell us what you are planning, we can decide whether it’s useful to delay the release and make changes to our codebase to ease packaging for you.

Vale, Quintus

muammar commented 9 years ago

@Quintus I prepared the pull request you can check it at https://github.com/Secretchronicles/TSC/pull/448. I think the final release should not be delayed. To get everything done to introduce tsc in debian, it will take a while.

Quintus commented 9 years ago

Okay. Thank you for your effort!

Vale, Quintus