Relocate JmeSurfaceView to com.jme3.view.surfaceview package

Pavl_G · April 23, 2022, 10:30pm

Hello fellows,

I just realized that i wasn’t following the packaging naming conventions on com.jme3.app.jmeSurfaceView and in addition to that the location of the jmeSurfaceView package inside com.jme3.app is wrong !

com.jme3.app → contains the jMonkeyEngine basic application components and application vital states that are essential to run a jme3 application.

JmeSurfaceView : is a android.view.RelativeLayout that wraps and starts a jMonkeyEngine app, but it doesn’t do that directly, it basically in a bad need of an android context (so basically it infers the rendering to GlSurfaceView within an android context (activity for example), if no context, then you cannot start a jme app using JmeSurfaceView).

So, i would like to refractor and relocate the package under a new location : com.jme3.view.surfaceview.

Q: So, why view.surfaceview, why not just view or surfaceview ?
A: All android native views live under android.view, except some special action views like android.opengl.GlSurfaceView and android.webkit.WebView for instance…So, putting JmeSurfaceView inside com.jme3.view.surfaceview would be relevant and would open the way for other chances to integrate more android views with jme.

Downsides :

This is a breaking change as @sgold states, so i would like to take the consent from the people who actively use JmeSurfaceView, is it okay to relocate the package ??

If you have other suggestions or objections, let me know too…

If you are using JmeSurfaceView on android, let me know…

pspeed · April 24, 2022, 1:04am

Under what conditions is this a breaking change?

I don’t do android development yet so I don’t know what apps have to refer to specifically versus what is hidden behind JME. What are the use-cases for an application referring to this package directly?

Or is that the standard way?

Pavl_G · April 24, 2022, 6:03am

JmeSurfaceView is a relative layout component, so it will appear in android native layout files (when writing a custom layout together with android GUI), example of usecase, this app (the gui is a native android gui – android views) :

And this is the layout file :

github.com

Monkey-Droid/DBTraining/blob/435105e9d19a23ee9dd4c685a182d25d5cd25be8/app/src/main/res/layout/fragment_prefacescreen.xml#L7


      
          <?xml version="1.0" encoding="utf-8"?>
          <RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
              xmlns:app="http://schemas.android.com/apk/res-auto"
              android:layout_width="match_parent"
              android:layout_height="match_parent">
          
          <com.jme3.app.jmeSurfaceView.JmeSurfaceView
              android:id="@+id/jmeSurfaceView"
              android:layout_width="match_parent"
              android:layout_height="match_parent"
              />
              <androidx.appcompat.widget.Toolbar
                  android:id="@+id/toolbar"
                  android:layout_width="match_parent"
                  android:layout_height="wrap_content"
                  android:background="@drawable/toolbar_background"
                  app:navigationIcon="@drawable/ic_baseline_menu_24"

So, com.jme3.app.jmeSurfaceView.JmeSurfaceView would appear in src package imports and layout files (if the user used it within a layout).

EDIT :

just in case, if an app is broken because of this, two files only may break, the android activity file and the layout file (if exists) and there must be an only one surface view for an android game.

Pavl_G · April 24, 2022, 6:06am

And you may do this, just to use androidx packages (if you donot want to incorporate android gui, you donot have to, you can use lemur).

capdevon · April 24, 2022, 11:17am

Honestly I’d be more inclined to correct the error and rename the package as Pavl suggested. The functionalities are the same and it takes very little to recompile an application. We all make mistakes, but this should be solved because a good programmer seeing such a thing would think that the whole engine is of low quality and would not use it. I appreciate Pavl’s contribution and support on Android, but the quality of an engine can also be seen in the details. A description of the change in the release notes of the next version will easily help programmers understand what to do to recompile their application if they have used the class.

Pavl_G · April 24, 2022, 5:29pm

Thank you, i am trying to do my best, so i will take this as an approval ? I was waiting to see if someone really uses the feature…

Again, JmeSurfaceView is not a thing used with jme3 game development patterns and appstates…its just for rendering which would be implemented only once (in one class file).

EDIT: I will provide the proper docs on package-info and the proper release notes to ease the migration…

capdevon · April 25, 2022, 8:31am

Yes, I support your initiative. Reading some documentation the most appropriate package name for me would be com.jme3.widget since the JmeSurfaceView class extends the android.widget.RelativeLayout class, and it is used in the layout xml configuration file (located in the path com/res/layout) as shown by you in the example.

It would become like this:
com.jme3.app.jmeSurfaceView.JmeSurfaceView → com.jme3.widget.JmeSurfaceView

Pavl_G · April 25, 2022, 9:49am

Yeah, i was to say that too, but take a look at android packages (on android there is a different between a widget and view).

From android docs :

andorid.widget package info → The widget package contains (mostly visual) UI elements to use on your Application screen. You can also design your own.
android.view package info → Provides classes that expose basic user interface classes that handle screen layout and interaction with the user (and this is the case with JmeSurfaceView, it provides basic ui for a SimpleApplication game, it also wraps the jme settings, update the jme game, init and destroy audio and jme app context).

So, widget are mostly visual and may not expose a direct functionality tho.

You can also find the android.opengl.GlSurfaceView parent (the SurfaceView class) under android.view.SurfaceView…so that why i suggested we may use com.jme3.view, what do you think ?

Android view package :
https://developer.android.com/reference/android/view/package-summary
Android widget package :
https://developer.android.com/reference/android/widget/package-summary

EDIT :
From my opinion : If we are going to contribute to the android open source project : then we would place our new views inside android.widget package, because they aren’t vitally essential to run the android application…,…in case of jme, JmeSurfaceView is an essential view to run the engine ! so being on com.jme3.view.surfaceview would be a better option i think…

capdevon · April 25, 2022, 10:24am

ok, but why view.surfaceview? It seems redundant to me. I see that in the jme3-android module the most used naming convention is com.jme3.xxx.android. What do you think about com.jme3.view.android? It seems to me more in line with the architecture of the module.

So it would become:
com.jme3.app.jmeSurfaceView.JmeSurfaceView → com.jme3.view.android.JmeSurfaceView

Pavl_G · April 25, 2022, 10:26am

JmeSurfaceView has some associated interfaces that cannot be floating around freely in the main view package (i imagine someone or even me trying some later time to add a new extension view for jme, what would be the case if we let JmeSurfaceView classes float around ?).

EDIT :
This usage to discriminate android components in these respective packages, the view appears only on android development so using android is redundant.

So, for example
com.jme3.input has a lot of things in jme3-core, so putting android only packages inside it is relevant and the same for iOS, but view is special for android.

Your opinion shoudl have been applied on app package btw.

capdevon · April 25, 2022, 12:08pm

Ok you convinced me. Go for com.jme3.view, simple solution with minimal impact. It is understood that all interfaces (OnExceptionThrown, OnLayoutDrawn, OnRendereCompleted, OnRenderStarted) are included in the same package as JmeSurfaceView as it is already now.

So it would become:

com.jme3.view

JmeSurfaceView
OnExceptionThrown
OnLayoutDrawn
OnRendereCompleted
OnRenderStarted

Ali_RS · April 25, 2022, 12:31pm

Agreed

Pavl_G · April 25, 2022, 12:59pm

Great, thank you @capdevon @Ali_RS

Pavl_G · May 1, 2022, 1:07am

This is a quick migration tutorial about migration, two files are affected :

Android Activity.
Android Activity Layout XML file (it may not be affected if the user has used the JmeSurfaceView on the activity only).

Pavl_G · May 4, 2022, 11:27am

A PR is submitted for approval, please take a look @capdevon @Ali_RS @pspeed @sgold :

Note, this PR includes commits related to PR ##1805 as its a sub-branch of java-docs improves, those commits will remove the conflicts between the 2 branches.

BharSat · May 5, 2022, 7:37am

Your new opinion, of android.widget.JmeSurfaceView would make more sense, but as @sgold said, it is a breaking change.

Pavl_G · May 5, 2022, 7:41am

Yeah as described above, the new class should be under com.jme3.view.surfaceview.JmeSurfaceView awaiting for review and approval, all the intended changes are documented inside a package-info file.