search

lucasstarsz / Slope-ECS

An Entity Component System written in Java, based on Austin Morlan's C++ ECS.
MIT License
14 stars 1 forks source link

Set Slope.ECS.Examples module to open #15

Closed nicvst closed 3 years ago

nicvst commented 3 years ago

Fixes #14

lucasstarsz commented 3 years ago

Thanks for pointing that out -- I completely forgot that modules need to access others.

However, I'm not a fan of the way you achieved the fix.

image An open module certainly works fine, but it then opens up all the parts of the example to the ECS. This is fine, but isn't really necessary. With Java modules, the open keyword (as you already know) allows other modules/classes full reflective access to the section(s) specified as open. Instead of opening up the entire module for reflective access, I would much rather just open up what's necessary. I did some experimenting, and opening up just the examples.guessinggame.systems package to the slope.ecs module solves the problem:

module Slope.ECS.examples {
    requires slope.ecs;
    opens examples.guessinggame.systems to slope.ecs;
}

(For more information on opening packages rather than classes, I really like this StackOverflow answer.)

I've no other complaints. Thanks again for noticing this; I will look into reconfiguring how the classpaths get set in my Gradle scripts so that issues like this show up properly as well.

nicvst commented 3 years ago

My worry lies with the Guessing Game as example project. Opening up modules on a per-package basis leaves users to export packages until their code compiles; is it clear which packages require opening up in a user project?

I'll update the PR.

lucasstarsz commented 3 years ago

That is a very good point, to be honest. One I'd little considered before making that comment up there! Well, I suppose it'd be one or the other -- if we go along with opening specific packages up then it'd need to be specified in the ruleset for making examples... once that exists 😅

To be very honest with you, either or works at the point. Whatever you do decide to do, I'll commit it and make note of it in a few hours. Just make sure you're sure of it.

nicvst commented 3 years ago

I've committed a change that replaces the example module configuration to your suggestion. Providing both options as suggestions and using the more restrictive approach ourselves would probably be the best course of action.

lucasstarsz commented 3 years ago

I agree, that should suffice. If anything comes up about this in the future, I'll refer back to this. Thanks.