junkdog / artemis-odb

A continuation of the popular Artemis ECS framework
BSD 2-Clause "Simplified" License
779 stars 113 forks source link

Changes in constructor visibility breaking gwt build. #137

Closed DaanVanYperen closed 10 years ago

DaanVanYperen commented 10 years ago

Some visibility changes are making my GWT build fail. Which may or may not be caused by me possibly needing some more sleep.

  [ERROR] Errors in 'E:\GitHub\arktrail\html\build\gwt\gen\com\artemis\gwtref\client\IReflectionCacheGenerated.java'
     [WARN] Line 8660: Referencing deprecated method 'com.artemis.systems.DelayedEntityProcessingSystem.restart'
     [ERROR] Line 8129: The constructor ComponentManager(int) is not visible
     [ERROR] Line 8178: The constructor ComponentType(Class<? extends Component>, int) is not visible
     [ERROR] Line 8268: The constructor EntityManager(int) is not visible

It's generating code like this in the generated reflection cache.

    private static com.artemis.EntityManager m106(int p0) {
  return new com.artemis.EntityManager(p0);
  }

If anyone can verify it's GWT that's broken and not me I can put some time in expanding the reflection generator. ;)

junkdog commented 10 years ago

I'll look into this tomorrow - i know i've broken some stuff with recent commits; it should mostly be a matter of copy-pasting the affected classes

Some visibility changes are making my GWT build fail. Which may or may not be caused by me possibly needing some more sleep.

[ERROR] Errors in 'E:\GitHub\arktrail\html\build\gwt\gen\com\artemis\gwtref\client\IReflectionCacheGenerated.java' [WARN] Line 8660: Referencing deprecated method 'com.artemis.systems.DelayedEntityProcessingSystem.restart' [ERROR] Line 8129: The constructor ComponentManager(int) is not visible [ERROR] Line 8178: The constructor ComponentType(Class<? extends Component>, int) is not visible [ERROR] Line 8268: The constructor EntityManager(int) is not visible

It's generating code like this in the generated reflection cache.

private static com.artemis.EntityManager m106(int p0) {

return new com.artemis.EntityManager(p0); }

Anyone else having GWT issues?

— Reply to this email directly or view it on GitHub https://github.com/junkdog/artemis-odb/issues/137.

DaanVanYperen commented 10 years ago

Spotted this in the GDX reflection fork: https://github.com/libgdx/libgdx/commit/0e8fc838d576af722eaa1f37b9d32d8049138566 -

Might be more worth porting: https://github.com/libgdx/libgdx/commits/master/backends/gdx-backends-gwt/src/com/badlogic/gwtref/gen/ReflectionCacheSourceCreator.java

Edit: Would save you the headache maintaining multiple versions.

DaanVanYperen commented 10 years ago

I'll apply the fix on my fork, unless you happen to be working on it right now!

junkdog commented 10 years ago

No, go right ahead! You have commit rights to main repo btw.

On Fri, Sep 12, 2014 at 11:15 AM, Daan van Yperen notifications@github.com wrote:

I'll apply the fix on my fork, unless you happen to be working on it right now!

— Reply to this email directly or view it on GitHub https://github.com/junkdog/artemis-odb/issues/137#issuecomment-55379089.

DaanVanYperen commented 10 years ago

That's madness! Why would you do that! I could be a serial killer.

junkdog commented 10 years ago

I feared someone would murder me during my vacation.

On Fri, Sep 12, 2014 at 11:22 AM, Daan van Yperen notifications@github.com wrote:

That's madness! Why would you do that! I could be a serial killer.

— Reply to this email directly or view it on GitHub https://github.com/junkdog/artemis-odb/issues/137#issuecomment-55379706.

DaanVanYperen commented 10 years ago

I might murder gwt, it's giving me a headache.

junkdog commented 10 years ago

Couldn't one just replace the offending files, opening up the constructors? Like in ComponentType, which has a more restrictive constructor in non-emulated artemis.

DaanVanYperen commented 10 years ago

My headache involved exactly that file. Wasn't aware of the emu version so I kept running into a missing field error when it clearly existed! (I thought it was some reserved name or visibility issue, derp.).

Got it all worked out now though. XD

junkdog commented 10 years ago

My headache involved exactly that file. Wasn't aware of the emu version so I kept running into a missing field error when it clearly existed! (I thought it was some reserved name or visibility issue, derp.).

Ouch, I was thinking before about having some fail-safe in place against files diverging (rather being left behind): I'm not sure such a tool/plugin is available, but it could be something as simple as linking hashes between emulated-original.

edit: if it's implemented as a maven plugin, the plugin could abort compilation when it notices that the emulated md5 (or whatever) doesn't match the expected checksum in artemis.

DaanVanYperen commented 10 years ago

This was developer error on my part. The ported tests should make you aware of any issues anyway!